Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Description • 15 years ago

We want to disable automatic scaling for now. When automatic scaling is reenabled, it should depend on platform settings such as "UI Resolution" on Mac.

We should get the true DPI for the screen and report it on all platforms.

I also think we should get the DPI through an API on nsIWidget, so that it can be per-widget ... i.e., creating a window on a new screen should get the right DPI.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 1 • 15 years ago

(In reply to comment #0)
> I also think we should get the DPI through an API on nsIWidget, so that it can
> be per-widget ... i.e., creating a window on a new screen should get the right
> DPI.

I don't know that this is a good idea -- what do you do when the window moves to another screen?  Or when it straddles two screens?  Or are you just referring to linux without xinerama or something, where an X Screen is isolated from all others?

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 2 • 15 years ago

(In reply to comment #1)
> I don't know that this is a good idea -- what do you do when the window moves
> to another screen?  Or when it straddles two screens?

Short-term answer: you lose, in that case. But we still get things right when you open a new window on a particular screen, or even reload a document while the window is on the right screen. We don't need to make the best the enemy of the good.

Long-term answer: it wouldn't actually be that hard to detect when the window has moved from one screen to another (or when the screen that the window is "mostly on" has changed) and fire some kind of event to perform a dynamic DPI change. We already support dynamic DPI changes when the layout.css.dpi pref changes. This could also handle the case when the screen DPI changes due to a mode change or other screen reconfiguration.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 3 • 15 years ago

This patch makes layout.css.devPixelsPerPx default to 1.0, which means nsIWidget::GetDefaultScale() will be ignored even if it gets implemented. I plan to file followup bugs to implement GetDefaultScale on each platform; having its usage prefed off by default will make it easier to implement GetDefaultScale incrementally.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 4 • 15 years ago

Adds code to implement nsIWidget::GetDPI on Mac, Windows and X.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 5 • 15 years ago

Remove gfxPlatform::GetDPI() now that it's no longer needed. For some Pango/GTK/Qt font stuff we still need some notion of DPI, but I believe this is really looking for "preferred font size" data. Anyway, I'm leaving behaviour unchanged there; Qt just uses 96 (since gfxQtPlatform doesn't implement InitDisplayCaps) and for GTK, gfxPlatformGtk::GetDPI does what gfxPlatformGtk::InitDisplayCaps used to do.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 6 • 15 years ago

Also does some refactoring. I guess there is still some question about whether we should do this. I think we should.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 7 • 15 years ago

There is also some question about whether we should do this.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 8 • 15 years ago

I think the above set of patches can land together to leave us in a good, shippable state, one that we can build on cleanly.

Supporting stuff like Windows' "font DPI" settings will require more work. I will file followup bugs to implement GetDefaultScale() on each platform, and also to add a media query feature to detect the device-pixels-per-CSS-pixel ratio. The latter is needed so that themes can offer artwork customized to particular resolutions, which we'll want before we enable scaling by default.

	Karl Tomlinson (:karlt)
	Comment 9 • 15 years ago

Comment on attachment 420228 [details] [diff] [review]
Part 1: Create new DPI APIs in nsIWidget

>+     * Return the physical DPI of the screen containing the window ...
>+     * the number of device pixels per inch.
>+     */
>+    virtual PRInt32 GetDPI() = 0;

Shouldn't this return double (or float)?

	Karl Tomlinson (:karlt)
	Comment 10 • 15 years ago

Comment on attachment 420229 [details] [diff] [review]
Part 2: Fetch DPI from the system

>+    Display *dpy = GDK_DISPLAY();
>+    int defaultScreen = 0;

= DefaultScreen(dpy);

>+    return NS_lround(DisplayHeight(dpy, defaultScreen)/heightInches);

Rounding may be significant with projectors, and a double return value gives a better hint as to what the function is calculating.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 11 • 15 years ago

I'll fix.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 12 • 15 years ago

This is basically just to annoy Vlad. MacOS makes it really easy to be notified when a window moves across screens, so take advantage of that and switch the DPI dynamically. The system chooses the screen that contains the most window area. The notification also fires when you change the screen's mode, which is nice.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 13 • 15 years ago

Made GetDPI return float.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 14 • 15 years ago

Make GetDPI return float and address Karl's comments

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 15 • 15 years ago

These tests check that pc/pt don't vary with DPI, and that mm/cm/in do vary (correctly) with DPI.

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 16 • 15 years ago

Note to self: We have a patch in bug 524919 that creates the true DPI for Qt. We could update attachment 420246 [details] [diff] [review] or land a followup:

https://bugzilla.mozilla.org/attachment.cgi?id=414047&action=edit

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 17 • 15 years ago

(In reply to comment #12)
> Created an attachment (id=420243) [details]
> Part 6: switch DPI dynamically when a window moves across screens on Mac
> 
> This is basically just to annoy Vlad. MacOS makes it really easy to be notified
> when a window moves across screens, so take advantage of that and switch the
> DPI dynamically. The system chooses the screen that contains the most window
> area. The notification also fires when you change the screen's mode, which is
> nice.

Grumble. I still think that's going to annoy users, especially if it's the default -- do any other mac apps behave this way?

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 18 • 15 years ago

Comment on attachment 420231 [details] [diff] [review]
Part 3: remove gfxPlatform::GetDPI

Looks fine, though I guess the Gtk bit is just a temporary workaround until we figure out the right way to do this with the crazy per-screen dpi stuff?

I wonder if it would be better to move the GetDPI call into gfxPangoFonts to make sure nothing else starts using it accidentally.  It's fine either way, though.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 19 • 15 years ago

In cases where it straddles, I don't think we should change the DPI unless the entire window has moved from one screen DPI to the other, or at least the vast majority (like, more than 85% of the window area). If I toss a window aside for a second in order to see something, I don't want my font sizes changing on me.

	Josh Aas
	Comment 20 • 15 years ago

Comment on attachment 420246 [details] [diff] [review]
Part 2 v2

>+  return (heightPx/scaleFactor)/(heightMM/25.4f);

You could probably put 25.4f in nsBaseWidget.h so we don't write it out 3 times. Maybe do the same for 96.0f.

I prefer space around operators, "x / y" instead of "x/y", that's how we've tried to do it everywhere else in Cocoa widgets. I think it is easier to read.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 21 • 15 years ago

(In reply to comment #18)
> (From update of attachment 420231 [details] [diff] [review])
> Looks fine, though I guess the Gtk bit is just a temporary workaround until we
> figure out the right way to do this with the crazy per-screen dpi stuff?

I dunno, there may be no way to do this right on X.

But picking an arbitrary screen and using its DPI for all windows is at least no worse from what we were doing before with a global DPI in gfxPlatform.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 22 • 15 years ago

(In reply to comment #17)
> Grumble. I still think that's going to annoy users, especially if it's the
> default -- do any other mac apps behave this way?

I don't know of any other Mac apps that use physical units.

(In reply to comment #19)
> In cases where it straddles, I don't think we should change the DPI unless the
> entire window has moved from one screen DPI to the other, or at least the vast
> majority (like, more than 85% of the window area). If I toss a window aside for
> a second in order to see something, I don't want my font sizes changing on me.

This only affects CSS dimensions specified in 'mm', 'cm' and 'in'. They are hardly ever used, so users aren't going to notice in general. Nor will the differences be very noticeable if your screens have similar DPI. If the DPIs are very different you'll probably be constantly annoyed that graphics spanning the screen boundary don't line up. The cases where this might be annoying are tiny tiny edge cases IMHO.

(In reply to comment #20)
> (From update of attachment 420246 [details] [diff] [review])
> >+  return (heightPx/scaleFactor)/(heightMM/25.4f);
> 
> You could probably put 25.4f in nsBaseWidget.h so we don't write it out 3
> times. Maybe do the same for 96.0f.
> 
> I prefer space around operators, "x / y" instead of "x/y", that's how we've
> tried to do it everywhere else in Cocoa widgets. I think it is easier to read.

OK.

	Karl Tomlinson (:karlt)
	Comment 23 • 15 years ago

(In reply to comment #21)
> (In reply to comment #18)
> > Looks fine, though I guess the Gtk bit is just a temporary workaround until we
> > figure out the right way to do this with the crazy per-screen dpi stuff?
> 
> I dunno, there may be no way to do this right on X.
> 
> But picking an arbitrary screen and using its DPI for all windows is at least
> no worse from what we were doing before with a global DPI in gfxPlatform.

gfxPlatformGtk::GetDPI is a logical font dpi (just for converting Pango-points/pixels).  There is only one value for all monitors on the Screen.
(Mozilla only runs on one Screen.)

nsIWidget::GetDPI is the physical dpi.  It looks like that information would be available for each monitor through the RANDR extension (newer versions at least), but the existing info from the entire Screen is much simpler to use and is a good start.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 24 • 15 years ago

(In reply to comment #20)
> (From update of attachment 420246 [details] [diff] [review])
> >+  return (heightPx/scaleFactor)/(heightMM/25.4f);
> 
> You could probably put 25.4f in nsBaseWidget.h so we don't write it out 3
> times. Maybe do the same for 96.0f.

I'll add MM_PER_INCH_FLOAT to nsCoord.h. I won't do anything for 96.0f, since it's an arbitrary default and there's no real reason it needs to be consistent across widget implementations.

> I prefer space around operators, "x / y" instead of "x/y", that's how we've
> tried to do it everywhere else in Cocoa widgets. I think it is easier to read.

Will do.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 25 • 15 years ago

Amusingly, this fixes a bug in SVG.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 26 • 15 years ago

Accessibility tests failed because accessibility's font-size text attribute returns values in pt, and accessibility was using the screen DPI to compute the pt, which no longer matches CSS pt when pt is non-physical. This patch fixes accessibility to report pt defined as 4/3 of a CSS pixel. I think this makes more sense for accessibility anyway; it doesn't make much sense to me for the font size reported by accessibility to depend on the DPI of the user's screen, especially if the user can't see the screen...

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 27 • 15 years ago

Some print reftests failed because they make assumptions about pts and inches. It's not easy to remove these assumptions because page dimensions and margins are defined in terms of inches and twips. So this patch just forces the DPI to 96 in DocumentViewerImpl::SetPageMode.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 28 • 15 years ago

Some other tests (mostly written by fantasai :-) ) assume there are always 72 points per inch. This changes those tests to use pt everywhere instead of inches, except for test_unit_length where we just temporarily force the DPI to 96.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 29 • 15 years ago

We probably need to do something to ensure that pt and pc have their correct physical interpretation when printing. I guess we should ensure that px is exactly 1/96 of an inch.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 30 • 15 years ago

Comment on attachment 420412 [details] [diff] [review]
Part 10: Force DPI for reftest-print to 96

This may not be necessary depending on how the DPI discussion evolves.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 31 • 15 years ago

There's an evolving discussion on www-style about what we should do here.

Partly thanks to Jonathan, I'm coming around to the view that we should redefine all existing units to be "non-physical" in non-print contexts, and create a new unit (say, "truemm") to be a physical unit in all contexts. This would let authors use in, cm, etc to get the correct dimensions when printed but still be usable on a variety of screens.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 32 • 15 years ago

The discussion on www-style seems to have mostly converged on redefining all existing units to be non-physical for non-print media.

It's less clear whether there will be a true physical unit or what it would be called. Since we have a clear use-case for chrome, I suggest we create our own new unit "mozmm".

	David Baron :dbaron:
	Comment 33 • 15 years ago

Comment on attachment 420410 [details] [diff] [review]
Part 8: Define MM_PER_INCH_FLOAT and use it

I suppose the loss in precision in the places operating on doubles isn't a real problem?

(Are there any tests about those places in SVG code that were using 24.4 instead of 25.4 for 'pc' units?)

r=dbaron

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 34 • 15 years ago

David, does the plan in comment #32 sounds OK to you? If so, I'll implement it. That would obsolete parts 4 and 5, and make part 11 unnecessary.

	David Baron :dbaron:
	Comment 35 • 15 years ago

Comment on attachment 420245 [details] [diff] [review]
Part 1 v2

sr=dbaron

	David Baron :dbaron:
	Comment 36 • 15 years ago

Comment on attachment 420243 [details] [diff] [review]
Part 6: switch DPI dynamically when a window moves across screens on Mac

r=dbaron

	David Baron :dbaron:
	Comment 37 • 15 years ago

Comment on attachment 420234 [details] [diff] [review]
Part 5: Make picas a non-physical unit

r=dbaron

	David Baron :dbaron:
	Comment 38 • 15 years ago

Comment on attachment 420233 [details] [diff] [review]
Part 4: Make pt a non-physical unit

>+nscoord nsCSSValue::GetPixelLength() const
>+  case eCSSUnit_Point:
>+    return nsPresContext::CSSPixelsToAppUnits(mValue.mFloat*4/3);

Maybe "* (4.0/3.0)" instead of "*4/3"?

>   PRBool    IsRelativeLengthUnit() const  
>-    { return eCSSUnit_EM <= mUnit && mUnit <= eCSSUnit_Pixel; }
>+    { return eCSSUnit_EM <= mUnit && mUnit <= eCSSUnit_RootEM; }
>+  PRBool    IsPixelLengthUnit() const
>+    { return eCSSUnit_Point <= mUnit && mUnit <= eCSSUnit_Pixel; }

Add a comment that what the spec calls relative length units is, for us, split between relative length units and pixel length units.  I like the new terminology better, but we should have a comment saying that it's different from CSS 2.1.

>-    zoom = aFontData.mSize.IsFixedLengthUnit() ||
>-           aFontData.mSize.GetUnit() == eCSSUnit_Pixel;
>+    zoom = aFontData.mSize.IsLengthUnit() &&
>+      !aFontData.mSize.IsRelativeLengthUnit();

Please keep the indentation style matching the surrounding code.

r=dbaron with that

	David Baron :dbaron:
	Comment 39 • 15 years ago

The CSS WG seems very against breaking the relationships between pt/pc/in/cm/mm.  The current proposal in the WG seems to be to require that 3pt == 4px (i.e., 96px == 72pt == 1in), and the UA can decide whether to peg the whole set of units to physical units or display pixels.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 42 • 14 years ago

Comment on attachment 420412 [details] [diff] [review]
Part 10: Force DPI for reftest-print to 96

Not needed now that we're going to make inches and points a fixed multiple of CSS pixels on screen

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 43 • 14 years ago

Comment on attachment 420413 [details] [diff] [review]
Part 11: fix other tests that ssume a relationship between inches and pt

Not needed now that we're going to make inches and points a fixed multiple of CSS pixels on screen

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 44 • 14 years ago

Addressed review comments. You might want to reread the comments in nsCSSValue.h.

As in the previous version of the patch, this patch just makes points be tied to CSS pixels instead of physical, and refactors code a bit to make that easier. I will attach a new patch to make all the other current "physical" units also be tied to CSS pixels.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 45 • 14 years ago

Mobile people need this.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 46 • 14 years ago

As previously discussed, this makes all the CSS standard "physical" units be fixed multiples of CSS pixels, leaving only mozmm as an actual physical unit.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 47 • 14 years ago

I've also updated the other patches to trunk and updated part 7, the tests. I'll reattach that for informational purposes.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 49 • 14 years ago

Running tests I found one rather large issue I'd overlooked: nsPresContext has methods for converting between appunits and twips/inches/points, and they were still using physical units, which is mostly not what we want.

This patch renames those methods to CSSPointsToAppUnits, etc, and makes them static since they no longer depend on the device. This actually simplifies a some SVG code quite a bit.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 50 • 14 years ago

Note that part 10 converts all the use of twips/inches for print margins etc to use CSS twips/inches instead of physical units. I think that's fine since for actual printing, CSS units match their physical counterparts.

	David Baron :dbaron:
	Comment 51 • 14 years ago

Comment on attachment 454482 [details] [diff] [review]
Part 4 v2

I'm a bit concerned about the change in NS_*_TO_TWIPS NS_TWIPS_TO_* (via
the change in NSUnitsToTwips and NSTwipsToUnits) that makes them not
round anymore.  I'm inclined to say you should go through all the users
that aren't passing the result to nsPresContext::TwipsToAppUnits and
either (a) add the rounding or (b) convert to floats, or that you add a
new function for floats (overloading has serious danger of compilation
errors, though) for those callers that do use TwipsToAppUnits.  If you 
want to do the go-through-all-the-users, I'd prefer to see that as an 
additional patch (on top?).

Other than that this looks fine.

	David Baron :dbaron:
	Comment 52 • 14 years ago

Comment on attachment 454484 [details] [diff] [review]
Part 5 v2: make pc, in, cm and mm all fixed multiples of CSS pixels

>+  eCSSUnit_Pica         = 904,    // (float) 12 points == 16 CSS pixls

s/pixls/pixels/

r=dbaron

	David Baron :dbaron:
	Comment 53 • 14 years ago

Comment on attachment 454826 [details] [diff] [review]
Part 10: Fix nsPresContext and nsIDeviceContext unit-conversion methods to be explicit about using CSS logical units instead of physical units

I'm having a little trouble understanding the changes in 
nsThebesDeviceContext::CalcPrintingSize.  Are CSS and physical units 
interchangable in that case, since it's for printing?

I'm guessing it's also the case in the DocumentViewerImpl::InitInternal
changes, but there using CSS makes sense since it'll be faster.

r=dbaron

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 54 • 14 years ago

(In reply to comment #51)
> I'm a bit concerned about the change in NS_*_TO_TWIPS NS_TWIPS_TO_* (via
> the change in NSUnitsToTwips and NSTwipsToUnits) that makes them not
> round anymore.  I'm inclined to say you should go through all the users
> that aren't passing the result to nsPresContext::TwipsToAppUnits and
> either (a) add the rounding or (b) convert to floats,

I'll audit the callers to the _TO_TWIPS macros. For NS_TWIPS_TO and NSTwipsToUnits, I've only changed the parameter from an nscoord to a float, so I don't think anything needs to be checked there. Right?

(In reply to comment #53)
> I'm having a little trouble understanding the changes in 
> nsThebesDeviceContext::CalcPrintingSize.  Are CSS and physical units 
> interchangable in that case, since it's for printing?

Yes. I'll add a comment.

> I'm guessing it's also the case in the DocumentViewerImpl::InitInternal
> changes, but there using CSS makes sense since it'll be faster.

Right.

	David Baron :dbaron:
	Comment 55 • 14 years ago

(In reply to comment #54)
> (In reply to comment #51)
> > I'm a bit concerned about the change in NS_*_TO_TWIPS NS_TWIPS_TO_* (via
> > the change in NSUnitsToTwips and NSTwipsToUnits) that makes them not
> > round anymore.  I'm inclined to say you should go through all the users
> > that aren't passing the result to nsPresContext::TwipsToAppUnits and
> > either (a) add the rounding or (b) convert to floats,
> 
> I'll audit the callers to the _TO_TWIPS macros. For NS_TWIPS_TO and
> NSTwipsToUnits, I've only changed the parameter from an nscoord to a float, so
> I don't think anything needs to be checked there. Right?

Er, right.  I originally had a comment mentioning only that, and then I changed it for some reason.

So I think you should be looking at all the users of *_TO_TWIPS that pass it to something other than NS_TWIPS_TO_* or nsPresContext::TwipsToAppUnits.

Be careful to use MXR text search rather than identifier search if you're using MXR; otherwise you'll miss the .mm files.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 56 • 14 years ago

I added NS_POINTS_TO_INT_TWIPS and NS_INCHES_TO_INT_TWIPS and used them in various places that were calling _TO_TWIPS and expecting an integer ... pretty much all related to printing. The other sites were putting the result in a float.

	David Baron :dbaron:
	Comment 57 • 14 years ago

Comment on attachment 464696 [details] [diff] [review]
Part 11: Audit _TO_TWIPS macro usage

Maybe you want NSToCoordRound rather than NSToIntRound?  They are different.  I'm not sure it matters.

r=dbaron

	David Baron :dbaron:
	Comment 58 • 14 years ago

Comment on attachment 454482 [details] [diff] [review]
Part 4 v2

r=dbaron given part 11

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 59 • 14 years ago

I think I want NSToIntRound. NSToCoordRound is optimized but only works on values in the nscoord range.

dbaron, if you agree we should land this for FF4, can you give blanket approval to the patches here? A comment in the bug would suffice.

	David Baron :dbaron:
	Comment 60 • 14 years ago

Yeah, I think we should.  a2.0=dbaron

	Michael Ventnor
	Comment 61 • 14 years ago

(In reply to comment #18)
> Comment on attachment 420231 [details] [diff] [review]
> Part 3: remove gfxPlatform::GetDPI
> 
> Looks fine, though I guess the Gtk bit is just a temporary workaround until we
> figure out the right way to do this with the crazy per-screen dpi stuff?

From GDK 2.14 you do have functions for multi-monitor setups:

http://library.gnome.org/devel/gdk/stable/GdkScreen.html

gdk_screen_get_monitor_at_window()
and
gdk_screen_get_monitor_width_mm()

I can't find a signal for when the window's screen changes like on OSX. Maybe you need to listen for when the window moves and check if the monitor number changes.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 62 • 14 years ago

This seems to have been responsible for test failures, so it was backed out.

MacOSX Debug geolocation test failure:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1281715052.1281717855.27052.gz

MacOSX and MacOSX64 Debug test_popup failures:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1281715052.1281717285.24668.gz

It may also have been responsible for a Txul regression, not sure yet.

I can't reproduce those failures locally, but I can on tryserver. Not sure why I didn't see those failures when I ran these patches through tryserver before. Maybe because Tinderbox was flakey at the time and those results were reported late.

When I get a chance I will try bisecting the patch queue on tryserver. That's not expensive since I can disable all builds except for MacOSX Debug.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 63 • 14 years ago

Bisection on try-server shows that the test failures appear to have been caused by part 6, dynamic DPI changing on Mac. That's fine, we can leave that patch out.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 64 • 14 years ago

Relanded a couple of days ago:
http://hg.mozilla.org/mozilla-central/rev/e95f616c9ee9
http://hg.mozilla.org/mozilla-central/rev/a3b0261ab61f
http://hg.mozilla.org/mozilla-central/rev/3569f9c1e13f
http://hg.mozilla.org/mozilla-central/rev/6229ad7196d4
http://hg.mozilla.org/mozilla-central/rev/3e399932a0e4
http://hg.mozilla.org/mozilla-central/rev/3cfc1b1098e6
http://hg.mozilla.org/mozilla-central/rev/49429c0426d1
http://hg.mozilla.org/mozilla-central/rev/46d18f2dabe9
http://hg.mozilla.org/mozilla-central/rev/a9982342b435
http://hg.mozilla.org/mozilla-central/rev/585a75516573
http://hg.mozilla.org/mozilla-central/rev/6991b186ea72

There was a Tp4 regression Mac, which was fixed in bug 588664.

	Christopher Blizzard (:blizzard)
	Comment 65 • 14 years ago

Note for docs:

http://weblogs.mozillazine.org/roc/archives/2010/08/css_units_chang.html

	Eric Shepherd [:sheppy]
	Comment 66 • 14 years ago

Documented here:

https://developer.mozilla.org/en/CSS-2_Quick_Reference/Units

And linked to from Firefox 4 for developers.

	Felix Miata
	Comment 67 • 14 years ago

(In reply to comment #66)
Good to have that URL, but I'm disappointed to see there "In addition, Gecko has improved ability to correctly scale absolute lengths to the actual resolution of the rendering medium, whether that's the screen or paper." without either an explanation or a link to an explanation.

On every Linux I install, like many others do, I force DPI to be accurate, so I need to know how to make pt mean pt & cm mean cm instead of the perpetuation of the unfortunate legacy explained on http://blogs.msdn.com/fontblog/archive/2005/11/08/490490.aspx that lead to this "bug" and "fix". Until users universally can easily adjust the size of their own px, px values of more than 9 in author CSS should be rejected by all web browsers as invalid, and 96 as a standard should be relegated to history books.

Meanwhile, thanks for mozmm, which keeps my http://fm.no-ip.com/Auth/dpi-screen-window.html tool viable.

	Dominic Fandrey
	Comment 68 • 14 years ago

I suppose mentioning this here will reach the people who care about DPI issues. Firefox4 renders everything at 96 DPI, no matter what I do, layout.css.dpi is completely ignored.

Tracking the issue down I found that gfx/src/thebes/nsThebesDeviceContext.cpp uses the PR_MAX macro in an unsafe manner, i.e. with function calls as parameters, which leads to very weird effects on my system. The macro substitution causes each function to be called twice and apparently they somehow interfere with each other and set stuff to 0 instead of a sensible value.

The result was that dpi always resulted in 96, no matter what was set in layout.css.dpi.

After fixing the issue I found that thebes has all the correct data on screen resolution and units, but layout.css.dpi still had no effect.

So I went in search for a central change to change this and ended up changing GetPixelLength() in layout/style/nsCSSValue.cpp. The function is hard coded to 96 DPI. I changed the signature to take a presentation context as an argument (the function is only called in 2 place in the entire source, so this wasn't a big deal) and made it use the information provided by thebes.

The result is that everything scales as expected I created a little dpi-meter at http://www.home.hs-karlsruhe.de/~fado0001/testdpi.html and not only does my trusty ruler tell me that the units are rendered perfectly, changing layout.css.dpi adjusts there rendering on the fly. I don't even have to reload the page.

My fix can be found in bug 603880, comment 5. I hope you will review it and take it into consideration.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 69 • 14 years ago

Thanks for the info. I commented about the PR_MAX issues in bug 603880.

It is intentional that DPI values other than 96 (either automatically detected, or manually forced via layout.css.dpi) have little effect in Firefox 4. See http://weblogs.mozillazine.org/roc/archives/2010/01/css_absolute_le.html and http://weblogs.mozillazine.org/roc/archives/2010/08/css_units_chang.html. Since our new behavior matches the other major browsers and is necessary for many Web pages to be usable on devices like phones, we really have no choice. If you disagree with the decision, that's fine, but please express your disagreement elsewhere, perhaps on my blog, or on your own blog :-).

You can use mozmm if you really want a physical unit. Note that a mozmm-sized element that looks fine on a desktop screen is unlikely to be suitably sized for a phone, say, so the use-cases for mozmm are quite limited.

	Tom Edwards
	Comment 71 • 13 years ago

I cannot understand that comment, Robert.

> It is intentional that DPI values other than 96 have little effect in Firefox 4.

No it is not. From your second link: "Currently GetDefaultScale always returns 1.0 on all platforms, although on Mac we should set it to the system 'default UI scale' and on Windows we should set it based on the 'system font DPI'."

> our new behavior matches the other major browsers

No it does not. Increase your DPI and Windows and test Chrome, Firefox and IE. Chrome and IE scale things up correctly, while Firefox does not.

> and is necessary for many Web pages to be usable on devices like phones

I can understand the need for phones to have sizing hacks. But there is no reason to force the same hacks onto the desktop!

> You can use mozmm if you really want a physical unit.

Who said anything about wanting a physical unit? High DPI is used all the time by people with poor eyesight who want things *to appear larger*.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 73 • 13 years ago

> > our new behavior matches the other major browsers
> 
> No it does not. Increase your DPI and Windows and test Chrome, Firefox and
> IE. Chrome and IE scale things up correctly, while Firefox does not.

Bug 603880 is about automatically scaling when you increase the Windows "font DPI". That is a valid bug. This bug was about changing behavior in response to screen DPI changes. Other browsers do not do that, and we match that now.

> Who said anything about wanting a physical unit? High DPI is used all the
> time by people with poor eyesight who want things *to appear larger*.

Right, you want bug 603880.