975919 - HiDPI support for Linux/GTK3

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement)

Description

[Brion Vibber]

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.

Comment 1

[Brion Vibber]

Attachment: Screenshot of GTK 3 Firefox build running on Fedora 20 in HiDPI mode

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.

Comment 2

[Kyra]

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

Comment 3

[Andrew Overholt [:overholt]]

(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.

Comment 5

[Andrew Comminos [:acomminos]]

Attachment: Bug 975919 - Added support for HiDPI on GTK 3.10+

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

Comment 6

[Brion Vibber]

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.

Comment 7

[Kevin Brosnan [Ex-Mozilla]]

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

Comment 8

[Andrew Comminos [:acomminos]]

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.

Comment 9

[Karl Tomlinson (:karlt)]

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.

Comment 10

[Andrew Comminos [:acomminos]]

Attachment: Bug 975919 - Added support for HiDPI on GTK 3.10+

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.

Comment 11

[Brion Vibber]

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.

Comment 12

[Brion Vibber]

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 13

[Karl Tomlinson (:karlt)]

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.

Comment 14

[Andrew Comminos [:acomminos]]

Attachment: Updated GTK3 HiDPI patch

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 15

[Karl Tomlinson (:karlt)]

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.

Comment 16

[Karl Tomlinson (:karlt)]

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.

Comment 17

[Andrew Comminos [:acomminos]]

Attachment: Updated GTK3 HiDPI patch r2

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!

Comment 18

[Andrew Comminos [:acomminos]]

Attachment: Updated GTK3 HiDPI patch r3

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

Comment 19

[Karl Tomlinson (:karlt)]

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

Thank you!

Comment 20

[Martin Stránský [:stransky] (ni? me)]

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

Try run: https://tbpl.mozilla.org/?tree=Try&rev=79a90ea436c4

Comment 21

[Martin Stránský [:stransky] (ni? me)]

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

Comment 22

[Andrew Comminos [:acomminos]]

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).

Comment 23

[Martin Stránský [:stransky] (ni? me)]

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?

Comment 24

[Martin Stránský [:stransky] (ni? me)]

(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().

Comment 25

[Martin Stránský [:stransky] (ni? me)]

(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.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f842d2ba5600

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f842d2ba5600

Comment 28

[[e-mail]]

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.

Comment 29

[jzamitini]

Just for the record, this "may" be related to the doubling-of-size issue I'm seeing in Kubuntu 14.04, Qt: 4.8.6, KDE 4.13.3 at https://bugzilla.mozilla.org/show_bug.cgi?id=1186145 for any firefox greater than or equal to version 38 (but not in any firefox less than or equal to version 37).

Screenshot of FF37 & FF39 simultaneously pointed to http://devicepixelratio.com/ is below:
 http://i.imgur.com/FbN0Uu4.jpg

Comment 30

[jzamitini]

Since the problem still exists, I'm confused as to how this GTK DPI calculation bug can be considered closed, since it clearly still occurs on FF39 on my Lenovo Thinkpad W510 laptop:
https://bug1186145.bmoattachments.org/attachment.cgi?id=8636734

As you must know, up:down screen space on a laptop (especially an HD laptop) is PRECIOUS, so, for Firefox versions from and including version 38 to waste space, and to open up twice the size that Firefox versions 37 and below did, thereby making them almost unusable, means the bug still exists.

Here is my laptop configuration:
$ uname -a 
=> Linux 3.13.0-48-generic #80-Ubuntu SMP x86_64 GNU/Linux

$ kde4-config --version
=> Qt: 4.8.6
=> KDE Development Platform: 4.13.3
=> kde4-config: 1.0

$ lsb_release -d
=> Description:	Ubuntu 14.04.2 LTS

$ lspci
.... ....
01:00.0 VGA compatible controller: NVIDIA Corporation GT216GLM [Quadro FX 880M] (rev a2)

$ /usr/bin/nvidia-smi
Tue Jul 21 11:48:05 2015       
+------------------------------------------------------+                       
| NVIDIA-SMI 340.76     Driver Version: 340.76         |                       
|-------------------------------+----------------------+----------------------+
| GPU  Name        Persistence-M| Bus-Id        Disp.A | Volatile Uncorr. ECC |
| Fan  Temp  Perf  Pwr:Usage/Cap|         Memory-Usage | GPU-Util  Compute M. |
|===============================+======================+======================|
|   0  Quadro FX 880M      Off  | 0000:01:00.0     N/A |                  N/A |
| N/A   50C   P12    N/A /  N/A |    188MiB /  1023MiB |     N/A      Default |
+-------------------------------+----------------------+----------------------+

$ inxi -F
System:    Host: karl Kernel: 3.13.0-48-generic x86_64 (64 bit) Desktop: KDE 4.13.3 Distro: Ubuntu 14.04 trusty
Machine:   System: LENOVO (portable) product: 4318CTO version: ThinkPad W510
           Mobo: LENOVO model: 4318CTO Bios: LENOVO version: 6NET74WW (1.34 ) date: 10/27/2010
CPU:       Quad core Intel Core i7 CPU Q 720 (-HT-MCP-) cache: 6144 KB flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) 
           Clock Speeds: 1: 933.00 MHz 2: 933.00 MHz 3: 933.00 MHz 4: 933.00 MHz 5: 933.00 MHz 6: 1600.00 MHz 7: 933.00 MHz 8: 1600.00 MHz
Graphics:  Card: NVIDIA GT216GLM [Quadro FX 880M] 
           X.Org: 1.15.1 drivers: nvidia (unloaded: fbdev,vesa,nouveau) Resolution: 1920x1080@60.0hz 
           GLX Renderer: Quadro FX 880M/PCIe/SSE2 GLX Version: 3.3.0 NVIDIA 340.76 

Clearly the bug still exists, and clearly it is therefore not resolved, at least in FF39. Right?

Comment 31

[jzamitini]

Attachment: Screenshot, new user, new $HOME/.mozilla, side-by-side FF38 & above, versus simultaneous FF37 and below showing the bug STILL exists in FF39!

In Bugzilla bug #1186145, we show this GTK automatic dpi calculation is wrong by a factor of 2x on my Lenovo W510 Thinkpad laptop (see details in my comment today); so I can't see how this bug can be considered "fixed", at least not in FF39 and below, which I have tested extensively.

Comment 32

[Thomas Daede [:TD-Linux]]

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.

Comment 33

[Andrew Comminos [:acomminos]]

(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).

Comment 34

[jzamitini]

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.

Comment 35

[mattia.b89]

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

Comment 36

[Makoto Kato [:m_kato]]

(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.

Comment 37

[mattia.b89]

yes, I mean Text Scaling Factor... sorry for my mistake

so now I follow the 1251432 bug as you stated