Gtk+ 3 has added support for a high-DPI drawing model similar to Mac OS X's and HTML's, applying a scale factor between virtual 'pixels' and display pixels, and giving you a drawing surface that applies to the right monitor's density. See the 2013 GUADEC 2013 talk on the subject for some background: http://videos.guadec.org/2013/High%20res%20display%20support%20GNOME/ Porting the widget layer to Gtk+ 3 is tracked in bug 627699 and appears to still be ongoing. Note also that the Linux theme currently lacks HiDPI artwork; this is tracked in bug 967100.

Screenshot of GTK 3 Firefox build running on Fedora 20 in HiDPI mode. * Firefox build from http://copr-fe.cloud.fedoraproject.org/coprs/stransky/FirefoxGtk3/ * on a Retina MacBook Pro in Parallels, with GDK_SCALE forced to 2 in /etc/environment * note the Gnome Shell title bars are wrong sized -- that's unrelated bug * note the GNOME file manager window is displayed at double scale, with beautiful high-resolution icons and text rendering * note the Firefox GTK3 build is displayed at double scale, but everything's rendered at low resolution and comes out blurry * note that just adjusting layout.css.devPixelsPerPx to 2 doesn't "fix" it as it would with a GTK2 build -- it just makes everything twice the size it should be.

Doesn't bug 627699 block this and not the other way around (as currently listed)?

(In reply to kxra from comment #2) > Doesn't bug 627699 block this and not the other way around (as currently > listed)? Perhaps. But a gtk2 build works pretty well on my high DPI laptop running Fedora 20.

I've done some work on a naive solution for this bug on HiDPI versions of GTK that does the following: - Forces the window's cairo surface device scale to 1 - Stores mBounds as device pixels, not GDK virtual pixels - Implements GetDefaultScaleInternal for GTK 3.10+ - Scales GDK virtual pixels up and down where appropriate - Corrects other scale-related aspects such as font sizing. Any feedback would be greatly appreciated. I figured using a cairo device scale of 1 would synergize better with the Mozilla project's use of scaling (similar to what we do on OS X). Andrew

Thanks Andrew! Trying it out now on Fedora 21 alpha -- looks pretty awesome! It picked up the correct scaling factor automatically and looks nice and sharp (versus a current gtk3 build which is all blurry, or a gtk2 release build that requires manual config adjustment). Some of the theme artwork still needs updating, of course, but that's another story. :D One visual glitch I see after a few minutes' poking is that the blue highlight on the context menu sometimes leaves a 1-pixel blue line when mousing over the items quickly. Probably an off-by-one error somewhere.

I get a crash on startup adding this patch ./firefox: symbol lookup error: /home/kbrosnan/Downloads/firefox/libxul.so: undefined symbol: cairo_surface_set_device_scale Using Cairo 1.12.16 and gnome-shell 3.12.2

I didn't consider the case where GTK would be compiled with a cairo version that doesn't support HiDPI; it might be a good idea to update the patch with a GTK3_HIDPI macro or something similar that checks for GTK 3.10+ and a supported cairo version. I'll add that to the patch.

Thanks, Andrew. Mozilla's build machines use GTK+ 3.4 so it would be helpful if the presence of necessary symbols can be detected at runtime. Then people can download and run Mozilla builds and have the new features if supported by their system. Feel free to wait until I've found time to review your patch if you like. It might be a few days before I can do that. GTK+ will have a dependency on a certain cairo version, so that may reduce the number of checks required.

Here's an updated patch with some convenience methods for scaling, runtime detection of cairo scaling support, and minor cleanup. I haven't been able to locate the exact cause of that bug Brion- I expect it to be an issue with nsWindow::Invalidate, as incrementing the rect in that method by a small integer in each dimension resolves the issue.

I think the problem is with having odd pixel coordinates; when we convert the rect from device to GDK coordinates we lose precision and the default integer rounding is throwing things off. Some quick printf()s.... and I see for example an invalidation on this rect: 0 85 388 47 which gets converted to GDK coordinates: 0 42 194 23 which, if we were to convert back to device coordinates gives: 0 84 388 46 which leaves an extra pixel or so at the bottom of the rect that didn't get invalidated. I think the rect conversion needs to be tweaked here so if the X or Y coordinate gets rounded down, the width or height gets bumped up to cover the shifted area.

This seems to work for me: nsIntRect nsWindow::DevicePixelsToGdkCoords(nsIntRect rect) { int scale = GdkScaleFactor(); float x = (float)rect.x / scale, y = (float)rect.y / scale, width = (float)rect.width / scale, height = (float)rect.height / scale; // Round to GDK coordinates; and make sure we // grow the rect in case of rounding to cover all. float rx = floor(x), ry = floor(y), rwidth = ceil(width + (x - rx)), rheight = ceil(height + (y - ry)); return nsIntRect(rx, ry, rwidth, rheight); }

Comment on attachment 8501526 [details] [diff] [review] Bug 975919 - Added support for HiDPI on GTK 3.10+ This looks a good approach in general. Just some details to iron out. Sorry about the delay here. It's taken me quite some time just to understand Gecko's approach to window scaling. (In reply to Andrew Comminos from comment #5) > - Stores mBounds as device pixels, not GDK virtual pixels Yes, that seems consistent other platforms and fragments of documentation. > I figured using a cairo device > scale of 1 would synergize better with the Mozilla project's use of scaling > (similar to what we do on OS X). I agree. >+STUB(gdk_window_get_scale_factor) Can you find this symbol at runtime, with dlsym() please? That would mean that Mozilla builds can also use this feature where available. Perhaps add a moz_gdk_window_get_scale_factor() that returns 1 if the symbol is not available. This stub can then be removed. I'd like to see GTK_CHECK_VERSION(3,10,0) replaced with MOZ_WIDGET_GTK >= 3. >+// Newer versions of cairo allow manually setting the surface's device scale. >+// This is useful for avoiding auto-scaling of HiDPI coordinate systems. >+static inline bool CairoDeviceScaleSupported() { >+ return dlsym(RTLD_DEFAULT, "cairo_surface_set_device_scale") && !dlerror(); >+} I'm not confident that dlsym is efficient enough for repeated use like this. That could be solved by using a static variable in function scope to store the pointer, but then the function shouldn't be static inline, but defined in a .cpp file. NULL is not a valid symbol address for the function, and so there's no need to check dlerror. (dlerror might in fact return a previous error.) >+ // Scale fonts up on HiDPI displays. >+ // This would be done automatically with cairo, but we manually manage >+ // the display scale for platform consistency. >+ size *= aDevPixPerCSSPixel; This will produce the wrong results with GTK2 when the devPixelsPerPx is set to adjust CSS pixel sizes in line with system font sizes that are already scaled wrt Xft.dpi. It's also not quite right for GTK3 - See bug 852963. The best thing to do I think is to use gdk_screen_get_monitor_scale_factor(gdk_screen_get_default, 0). On X11, GDK always returns the same scale factor for all monitors. We can worry about a per window scale when/if Wayland becomes a target platform. gdk_screen_get_monitor_scale_factor is also only available with GDK 3.10, so it may be sensible to make a moz_gdk_screen_get_scale_factor function that just returns 1 if the symbol is not available including GDK2. Given gdk_window_get_scale_factor() always returns the same value, perhaps moz_gdk_screen_get_scale_factor can replace that, or GDKScaleFactor() could be a static method to provide this info. > NS_IMETHODIMP > nsWindow::ConstrainPosition(bool aAllowSlop, int32_t *aX, int32_t *aY) > { > if (mIsTopLevel && mShell) { >- int32_t screenWidth = gdk_screen_width(); >- int32_t screenHeight = gdk_screen_height(); >+ int width = GdkCoordsToDevicePixels(gdk_screen_width()); >+ int height = GdkCoordsToDevicePixels(gdk_screen_height()); I think there should also be a divisor of GetDefaultScale() included here, but that is an existing bug. nsXULWindow::Center() calls this function with so- called "display pixels", which are Gecko's device-independent pixels, matching device pixels multiplied by GetDefaultScale() usually. Perhaps this should use nsIScreen::GetAvailRectDisplayPix() but no need to worry about this in this patch. Just writing this down, while I can see (from "DevicePixels") something that should be fixed. >- geometry.min_width = mSizeConstraints.mMinSize.width; >- geometry.max_width = mSizeConstraints.mMaxSize.width; >+ geometry.min_width = DevicePixelsToGdkCoords( >+ mSizeConstraints.mMinSize.width); >+ geometry.max_width = DevicePixelsToGdkCoords( >+ mSizeConstraints.mMaxSize.width); Looks like RoundUp/Down and/or RoundIn/Out versions of DevicePixelsToGdkCoords() would be helpful. >+ int gdkX = DevicePixelsToGdkCoords(x); >+ int gdkY = DevicePixelsToGdkCoords(y); A GdkPoint DevicePixelsToGdkPointRoundDown(nsIntPoint) would be useful, saving two function calls to get the window scale. > gdk_window_get_origin(mGdkWindow, &x, &y); > } > >- return nsIntPoint(x, y); >+ return nsIntPoint(GdkCoordsToDevicePixels(x), >+ GdkCoordsToDevicePixels(y)); >- aRect.MoveTo(x, y); >+ aRect.MoveTo(GdkCoordsToDevicePixels(x), GdkCoordsToDevicePixels(y)); Similarly, nsIntPoint GdkPointToDevicePixels(GdkPoint). >- GdkRectangle rect; >- rect.x = aRect.x; >- rect.y = aRect.y; >- rect.width = aRect.width; >- rect.height = aRect.height; >+ nsIntRect rect = DevicePixelsToGdkCoords(aRect); > > LOGDRAW(("Invalidate (rect) [%p]: %d %d %d %d\n", (void *)this, > rect.x, rect.y, rect.width, rect.height)); > >- gdk_window_invalidate_rect(mGdkWindow, &rect, FALSE); >+ GdkRectangle gdkRect = { rect.x, rect.y, rect.width, rect.height}; >+ gdk_window_invalidate_rect(mGdkWindow, &gdkRect, FALSE); DevicePixelsToGdkRectRoundOut(), which would fix comment 12. >- nsIntRegion &region = exposeRegion.mRegion; >+ nsIntRect exposeRect = exposeRegion.mRegion.GetBounds(); >+ nsIntRegion region(GdkCoordsToDevicePixels(exposeRect)); This is OK for a prototype, but I think we need to keep the regions for efficiency. > nsWindow::OnSizeAllocate(GtkAllocation *aAllocation) > { > LOG(("size_allocate [%p] %d %d %d %d\n", > (void *)this, aAllocation->x, aAllocation->y, > aAllocation->width, aAllocation->height)); > >- nsIntSize size(aAllocation->width, aAllocation->height); >+ nsIntSize size(GdkCoordsToDevicePixels(aAllocation->width), >+ GdkCoordsToDevicePixels(aAllocation->height)); >+ > if (mBounds.Size() == size) > return; > >+ nsIntRect rect; >+ > // Invalidate the new part of the window now for the pending paint to > // minimize background flashes (GDK does not do this for external resizes > // of toplevels.) > if (mBounds.width < size.width) { >- GdkRectangle rect = >- { mBounds.width, 0, size.width - mBounds.width, size.height }; >- gdk_window_invalidate_rect(mGdkWindow, &rect, FALSE); >+ rect = nsIntRect(mBounds.width, 0, size.width - mBounds.width, >+ size.height); > } > if (mBounds.height < size.height) { >- GdkRectangle rect = >- { 0, mBounds.height, size.width, size.height - mBounds.height }; >- gdk_window_invalidate_rect(mGdkWindow, &rect, FALSE); >- } >+ rect = nsIntRect(0, mBounds.height, size.width, >+ size.height - mBounds.height); >+ } >+ >+ rect = DevicePixelsToGdkCoords(rect); >+ GdkRectangle gdkRect = { rect.x, rect.y, rect.width, rect.height }; >+ gdk_window_invalidate_rect(mGdkWindow, &gdkRect, FALSE); This isn't right because rect set in the first block is overwritten by the second block. Consider using nsWindow::Invalidate() here. >+ if (CairoDeviceScaleSupported()) { >+ // Disable auto-scaling on HiDPI devices, let mozilla manage it. >+ cairo_surface_set_device_scale(surf, 1, 1); >+ } We can't depend on the linker not to try to resolve this symbol. LD_BIND_NOW may be set and there is similar behavior in some tighter security situations, for example. However, this surf belongs to GTK and so I'm not comfortable with modifying it. I may work but it is depending too much on the implementation of system libraries. I expect it should be possible to multiply (or just set if it is a new context) the current transformation matrix of the drawing context with the scale to achieve the same result. That would also make finding the cairo_surface_set_device_scale symbol at runtime unnecessary. >+ int DevicePixelsToGdkCoords(int pixels); >+ nsIntRect DevicePixelsToGdkCoords(nsIntRect rect); >+ int GdkCoordsToDevicePixels(int coords); >+ nsIntRect GdkCoordsToDevicePixels(nsIntRect rect); Please use gint and GdkRect when referring to GDK units. The conversions to GDK units should specify whether they round up, down, our, or in as appropriate. Please use DevicePixelsToGdkCoord and GdkCoordToDevicePixels with singular "Coord" for the methods dealing with only one coord. I expect some reverse scaling will be required in nsNativeThemeGTK::DrawWidgetBackground() to draw native widgets at the right scale? Check boxes, radio buttons, and scrollbars in-particular usually do not scale to fill their allocations. And probably some other length units need adjusting somewhere between nsNativeThemeGTK and gtk3drawing.c. This is better handled separately anyway. Also nsScreenGtk may need to implement the DisplayPix versions, but that also is better handled separately.

Thanks for the feedback, Karl- attached is an updated patch as per your review. The patch has also been modified to apply scaling to OMTC windows and properly represent HiDPI displays in nsScreenGtk. Let me know if there's anything else missing. I've opted to keep the (now dynamically resolved) cairo_surface_set_device_scale call. Applying a transform of 1/GdkScaleFactor to the cairo context ends up getting discarded down the line for some windows in the 2d stack. Happy holidays.

Comment on attachment 8541380 [details] [diff] [review] Updated GTK3 HiDPI patch >diff --git a/widget/gtk/nsGtkUtils.h b/widget/gtk/nsGtkUtils.h >--- a/widget/gtk/nsGtkUtils.h >+++ b/widget/gtk/nsGtkUtils.h >+#include <dlfcn.h> Perhaps this was just left behind accidentally, but please only include headers in the particular files that require them. >+ static gint (*GdkScreenGetMonitorScaleFactorPtr)(GdkScreen*,gint) = >+ (gint (*)(GdkScreen*,gint)) dlsym(RTLD_DEFAULT, >+ "gdk_screen_get_monitor_scale_factor"); >+ if (GdkScreenGetMonitorScaleFactorPtr) { >+ GdkScreen *screen = gdk_screen_get_default(); >+ size *= (*GdkScreenGetMonitorScaleFactorPtr)(screen, 0); >+ } Gecko convention is to have a leading lower case letter for variable names and use captialized first letter for types. Here GdkScreenGetMonitorScaleFactorPtr could be sGdkScreenGetMonitorScaleFactorPtr where 's' indicates static. In the old days a typedef would be used to define the function type, to avoid stating the parameter type twice. Now C++ allows 'auto' as a type-specifier (but not a storage class specifier as in C), so I'd guess "static auto sGdkScreenGetMonitorScaleFactorPtr =" would fine here. Similarly elsewhere. >+++ b/widget/gtk/nsScreenGtk.cpp I suggest leaving nsScreenGtk changes to a separate patch or bug. If those are dropped from this patch, then the only major issue I'd like to see addressed before this is pushed is cairo_surface_set_device_scale(). Currently nsScreenGtk rectangles are only in GDK coords if there is a single screen and there is no available rectangle provided. Otherwise they are device pixels from X11. I think it will be simplest if mAvailRect and mRect are both stored in device pixels, converted if they come from gdk_screen_width/height. The ratio between device pixels and Gecko "display pixels" is affected by static nsIWidget::DefaultScaleOverride(), like nsIWidget::GetDefaultScale(), as with the conversions in the nsWindow::Move. If there is an override provided, then the display pixels won't necessarily match GDK coords. This allows, for example, setting a pref to scale web content by 1.5 instead of 2, to fit more content on the screen. contentsScaleFactor comes from bug 785667, and seems to have a very Mac-specific purpose. I suspect it doesn't quite mean the ratio between device pixels and Gecko "display pixels", but I don't think it is used on non-Mac platforms anyway. In general, GDK's scale factor should only be used when dealing with GDK types or functions, while GetDefaultScale() or similar is used for scaling in Gecko. (In reply to Andrew Comminos from comment #14) > I've opted to keep the (now dynamically resolved) > cairo_surface_set_device_scale call. Applying a transform of > 1/GdkScaleFactor to the cairo context ends up getting discarded down the > line for some windows in the 2d stack. I'm still not comfortable with modifying GDK's surface here. What if the non-null cairo_t cr path in GetThebesSurface is dropped and instead the other (null cr) path, assuming that gdk_x11_window_get_xid() gives us the right drawable, is used? That is what is happening for OMTC anyway. > nsWindow::DispatchEvent(WidgetGUIEvent* aEvent, nsEventStatus& aStatus) > { > #ifdef DEBUG > debug_DumpEvent(stdout, aEvent->widget, aEvent, > nsAutoCString("something"), 0); > #endif >+ // Translate the mouse event into device pixels. >+ aEvent->refPoint.x = GdkCoordToDevicePixels(aEvent->refPoint.x); >+ aEvent->refPoint.y = GdkCoordToDevicePixels(aEvent->refPoint.y); This catches most of the event coord issues. There are some remaining, but they need not be covered in this patch: The synthEvent code in OnMotionNotifyEvent() treats WidgetMouseEvent::refPoint as device pixels before it is converted from GDK coords in DispatchEvent(). InitButtonEvent(), OnScrollEvent(), and OnMotionNotifyEvent() convert child window coords using WidgetToScreenOffset, which is in device pixels, but the coords have not been converted to device pixels at this stage. > void nsWindow::SetSizeConstraints(const SizeConstraints& aConstraints) >+ geometry.min_width = DevicePixelsToGdkCoordRoundUp( >+ mSizeConstraints.mMinSize.width); >+ geometry.min_height = DevicePixelsToGdkCoordRoundUp( >+ mSizeConstraints.mMinSize.height); >+ geometry.max_width = DevicePixelsToGdkCoordRoundUp( >+ mSizeConstraints.mMaxSize.width); >+ geometry.max_height = DevicePixelsToGdkCoordRoundUp( >+ mSizeConstraints.mMaxSize.height); Round max values down, to avoid allowing greater values. I guess that could lead to max < min, but that could already happen and GTK will deal with that for us. > // Owen Taylor says this is the right thing to do! >- width = std::min(32767, width); >- height = std::min(32767, height); >+ width = std::min(32767, (int)std::ceil(GdkCoordToDevicePixels(width))); >+ height = std::min(32767, (int)std::ceil(GdkCoordToDevicePixels(height))); ceil is not necessary because GdkCoordToDevicePixels() returns int. That also removes the need for the (int) cast. Better update the comment now that these lines do more: "Owen Taylor says limiting to 32767 is the right thing to do!". >+gint >+nsWindow::DevicePixelsToGdkCoordRoundUp(int pixels) { >+ return NSToIntCeil(float(pixels)/float(GdkScaleFactor())); >+} The rounding from floating point arithmetic can produce surprising results with ceil. There should be no rounding when GdkScaleFactor() only ever returns a power of 2, but 3 might be a feasible value. Instead: gint scale = GdkScaleFactor(); return (pixels + scale - 1) / scale; Similarly in the other conversion functions, stick to integer arithmetic. >+nsWindow::GdkCoordToDevicePixels(gint coords) { >+ return coords * GdkScaleFactor(); s/coords/coord/ >+ virtual double GetDefaultScaleInternal(); Please add MOZ_OVERRIDE to the end of this declaration.

The difficulty with an alternative approach that used gdk_x11_display_set_window_scale(display, 1) to simplify GDK coord to device pixel conversion and then read scale factors from settings to apply within Gecko would be that native GTK dialogs such as the file picker would not know about the scale factor, unless they used a different display. That I think means that the approach in the patch here of converting to device pixels wherever GDK is involved is better.

Here's an updated patch with the fixes you mentioned, and the nsScreenGtk changes removed. I've removed the Cairo surface wrapping (the non-null path for cairo_t cr) as per your suggestion. I suppose it wouldn't have found much use anyway moving forward with OMTC. Thanks!

Sorry, a minor bug with nsWindow::DevicePixelsToGdkRectRoundOut was introduced in the last patch when switching to integer division. This should fix it.

Comment on attachment 8552272 [details] [diff] [review] Updated GTK3 HiDPI patch r3 Thank you!

Comment on attachment 8552272 [details] [diff] [review] Updated GTK3 HiDPI patch r3 Try run: https://tbpl.mozilla.org/?tree=Try&rev=79a90ea436c4

Try looks okay, at least the affected parts. Andrew, I guess we can commit the patch, right?

It should be alright, the failed tests appear to be recurring intermittant failures according to the tryserver's related bugs. I'm not too confident how that area of the stack could be affected by these changes as it stands. We could try another run to see if it was a clobber-related issue I suppose (I haven't been able to reproduce the test failures locally).

(In reply to Martin Stránský from comment #23) > Comment on attachment 8552272 [details] [diff] [review] > Updated GTK3 HiDPI patch r3 > > Karl, I just tested this one along the X11 scaling patch from Bug 1081142 > and the X11 scaling (gtk2 version) is broken. The coords calculation is > converted back to the unscaled resolution. Is that expected? Sorry, ignore that. I was confused by GdkScaleFactor() and GetDefaultScaleInternal().

(In reply to Andrew Comminos from comment #22) > We could try another run to see if it was a clobber-related issue I suppose > (I haven't been able to reproduce the test failures locally). Let's ask for the commit then, looks fine.

On my end, Firefox's per-page zoom doesn't respect gtk settings. The Firefox pages (settings, config, etc) and layout do scale, but browsing web sites, I do not see a scaling of the web pages.

Firefox is correctly using the GTK DPI here. You need to file a bug against GTK3 instead. This bug is only about respecting GTK settings. You can always force GDK scale by starting firefox with the environment variable GDK_SCALE=1. There is also the about:config option layout.dev.cssPixelsPerPx which can be non-integer.

(In reply to jzamitini from comment #30) > Clearly the bug still exists, and clearly it is therefore not resolved, at > least in FF39. Right? I don't think this is the correct bug to address your issue- this is for implementing support for GDK's scale factor, which was introduced in GTK 3.10 (and thus is not currently riding the trains, as current release builds link against GTK2).

Thanks guys for explaining that. It must be a bug somewhere in the XUL/GTK code, so, I'll let the existing bug 1186145 run its course. Thanks. Much appreciated your information.

Hi fellows, I think this bug has to be reopened since FF doesn't respect Scaling Factor. I open a topic in the Arch Linux forum (https://bbs.archlinux.org/viewtopic.php?pid=1613115). in which you can find all the useful information

(In reply to mattia.b89 from comment #35) > Hi fellows, > I think this bug has to be reopened since FF doesn't respect Scaling Factor. > I open a topic in the Arch Linux forum > (https://bbs.archlinux.org/viewtopic.php?pid=1613115). > in which you can find all the useful information Could you file a new bug for it? This bug is to support scaling-factor value of org.gnome.desktop.interface. It only allows integer. If gnome-tweak-tool allows non-integer value for scaling-factor, this is a bug of gnome-tweak-tool. And, If you say about text scaling factor, it is bug 1251432.

yes, I mean Text Scaling Factor... sorry for my mistake so now I follow the 1251432 bug as you stated