Closed Bug 674373 Opened 13 years ago Closed 12 years ago

[10.7] Support HiDPI mode on OS X Lion

Categories

(Core :: Widget: Cocoa, defect)

Product:
Core
Component:
Widget: Cocoa
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18

People

(Reporter: limi, Assigned: jfkthame)

References

(Depends on 2 open bugs, Blocks 3 open bugs)

Details

Attachments

(45 files, 33 obsolete files)

Screenshot showing FF10 HiDPI font rendering (compared to Safari 5.1)
56.86 KB, image/png
Details
add NSPrincipalClass entry
561 bytes, patch
smichaud
: review+
Details | Diff | Splinter Review
Screnshot: Firefox & Safari in 960x600 HiDPI
644.45 KB, image/png
Details
Screenshot of FF13 patched (left) and unpatched (right), 800x600 HiDPI mode
630.76 KB, image/png
Details
Screenshot of current build on Retina Macbook Pro
2.90 MB, image/png
Details
Screenshot of current nightly on Retina MacBook Pro with layers acceleration disabled
2.61 MB, image/png
Details
experimental patch to introduce screen points
426.45 KB, patch
Details | Diff | Splinter Review
dynamic resolution change handling
6.63 KB, patch
Details | Diff | Splinter Review
Wikipedia homepage in Safari (left) & Firefox try build (right) on Retina MacBook Pro
2.04 MB, image/png
Details
Zebra image in Safari (left), Firefox try (right) ACCELERATION DISABLED
3.84 MB, image/png
Details
Zebra image in Safari (left), Firefox try (right) ACCELERATION ENABLED
3.53 MB, image/png
Details
Using the test build in comment 72, top down: no accel, accel, safari.
2.15 MB, image/png
Details
OpenGL antialiasing HiDPI patch, updated to current trunk
5.48 KB, patch
Details | Diff | Splinter Review
screenshot of some text in Firefox on rMBP
972.63 KB, image/png
Details
screenshot of some text in Firefox and Safari for comparison
1.27 MB, image/png
Details
Screenshot showing that Web Developer's "Inspect" feature gets it wrong
617.90 KB, image/png
Details
Favicon does not appear next to URL
96.20 KB, image/png
Details
Comparison of pdf.js on Firefox and Safari
372.19 KB, image/png
Details
Screenshot of canvas drawing
24.40 KB, image/png
Details
Screenshot showing WebGL in low res
460.13 KB, image/png
Details
Screen Capture from Non-Retina Display
316.88 KB, image/jpeg
Details
Screenshot showing 2x fonts when tab dragged from HiDPI to non-HiDPI displays
582.85 KB, image/png
Details
Ghosts of caret
111.79 KB, image/png
Details
mov of issue
2.96 MB, application/octet-stream
Details
broken titletip box
19.91 KB, image/png
Details
Bad redraw when dragged from non-HiDPI to HiDPI display
416.34 KB, image/png
Details
image of small vertical scrollbar target
44.63 KB, image/png
Details
Nested Folder Scaling issues
397.07 KB, image/jpeg
Details
Issue with the Find bar
51.09 KB, image/png
Details
Second snapshot of the "find bar" issue
65.98 KB, image/png
Details
Low-res Chrome
50.63 KB, image/png
Details
Weird scaling issues on social buttons (in-content)
19.77 KB, image/png
Details
Screenshot of half-size Gmail after detaching external monitor
628.05 KB, image/png
Details
Half-size rendering of zoomed window after screen detach
913.38 KB, image/png
Details
Inactive colours are a bit weird
46.31 KB, image/png
Details
Screenshot of Gmail misrendered after tearing off tab from Retina screen to non-Retina screen
228.86 KB, image/png
Details
Screenshot of Gmail misrendered after tearing off tab from non-Retina screen to Retina screen
697.49 KB, image/png
Details
Try Server 50af84dce998 is running hi full res, rather than HiDPI
549.63 KB, image/png
Details
pt 2 - consistently use client bounds for layer sizing.
2.41 KB, patch
roc
: review+
Details | Diff | Splinter Review
pt 3 - remove unused nsBaseWidget::SetBounds method.
2.19 KB, patch
roc
: review+
Details | Diff | Splinter Review
pt 1 - Mac OS X native theme rendering support for HiDPI display.
10.46 KB, patch
smichaud
: review+
Details | Diff | Splinter Review
pt 4 - pass device-to-CSS pixel ration to LookAndFeel code when requesting a font style.
14.81 KB, patch
roc
: review+
Details | Diff | Splinter Review
pt 5 - provide a fullZoom API in nsIDOMWindowUtils, and use this rather than inferring zoom from CSS to device pixel ratio.
11.83 KB, patch
jfkthame
: review+
Details | Diff | Splinter Review
pt 6 - support HiDPI display in Cocoa widget code.
74.50 KB, patch
smichaud
: review+
Details | Diff | Splinter Review
Bad window rendering
3.62 MB, image/png
Details 
When running in HiDPI mode on OS X Lion, we should show double size icons, and make sure that the text is rendered properly.
What happens currently? I know we consider the screen's DPI, though I think we don't start adjusting for it until after some threshold (though aiui HiDPI is 2x the usual setting, so I'd assume that's enough to do it...)
I think we should start here by implementing nsChildView::GetDefaultScale() to return something other than 1.0 when appropriate.
Summary: Support HiDPI mode on OS X Lion → [10.7] Support HiDPI mode on OS X Lion
I should add that, funnily enough, Firefox, up to version 3.6.21, renders the fonts on webpages correctly under HiDPI. Starting with Firefox 4.0 UI fonts, and the fonts on webpages, are very pixelated under HiDPI. The same is true for Thunderbird: UI fonts, and the fonts in email messages, render correctly only until Thunderbird version 3.1.11, and not in subsequent versions.
This screenshot was taken on a HiDPI display (IBM T221) on OS X 10.7.3, on the left is Firefox 10.0.1, on the right you see Safari 5.1.3.

Safari looks absolutely stunning at 204 dpi.

With the advent of HiDPI Macbooks it would be great if Firefox were ready for it.
Yup, we tested this with Air Display earlier today too, it looks like this:

http://f.cl.ly/items/2z372F1l0H0o092a3M05/Screen%20Shot%202012-04-02%20at%204.03.20%20PM.png

Markus, do you have any idea why the window decorations are low-res even when running in HiDPI? I assume we copy them from the lower-res version since we want to support themes in the titlebar.

That we don't render fonts at the native resolution is interesting too.
(In reply to Alex Limi (:limi) — Firefox UX Team from comment #5)
> Markus, do you have any idea why the window decorations are low-res even
> when running in HiDPI?

I haven't looked at this at all yet, but maybe there's a per-application switch that we need to flip before OS X gives us HiDPI window decorations.
Note, 10.7.3 will show HiDPI modes if you are plugged into a HDTV via HDMI. I use 960x540 HIDPI exclusively. This works much better than using a blurry 720p mode on a 1080p display just to get a UI that's usable at a distance. Now if only I could use Firefox instead of just Safari. I don't think this is a feature you should wait on adding. Not only useful for TVs but also for people with poor vision.
If the rumours are correct, Apple may be releasing HiDPI computers in June.

I think we should at least render all widgets and cursors at 4x resolution and get them pushed. Just so we are most the are ready for when and not if Apple announce HiDPI. It will look good for Mozilla if they're one of the first third party developers to support HiDPI. 

Is there any API switch to tell the OS that the app is HiDPI supported in OS X Lion?
Through much trial and error I've found out that the NSPrincipalClass key needs to be present in our app bundle's Info.plist file, otherwise HiDPI support is turned off.
I wouldn't call that an API, but at least now we know how to enter HiDPI mode at all.

Googling "HiDPI NSPrincipalClass" tells me that the Chrome guys had the same problem: http://codereview.chromium.org/10069029
With this patch, at least the window decorations are high resolution now. With hardware acceleration off, font and border rendering is already high resolution, too.
Patch doesn't work for me; window decorations etc remain pixelated.
In comment #7 HIDPI Fan mentions one way to test HiDPI mode.  Are there others?

I need detailed steps! :-)
You can enable HiDPI resolutions using 'Quartz Debug' in Xcode 4.1 on 10.7.3. Select 'Window->UI Resolution'. In my computers the setting was checked so I had to disable, logout, enable, logout. After this HiDPI resolutions appear in System Preferences. I believe this will simulate HiDPI displays for development.

Currently unpatched we run severely pixelated.
Here's another screenshot of (unpatched) Firefox and Safari in HiDPI mode, in which I'd say that they look equally pixelated.  (You have to zoom in on the image to see this.)

This was on a non-Apple monitor (an NEC LCD2190UXp display) -- which may explain the difference from Benoit's test.

I used Grab to take the screenshot, and Preview to convert it to PNG.
(Following up comment #15)

I tested on OS X 10.7.4.
I don't think that you are running the same HiDPI mode as I was. Note that the top right 'full screen' button is 15x15 on your screenshot and 30x30 on mine.
Oops, now I see my mistake.

I did test in what OS X told me was a genuine 800x600 HiDPI mode.  But it was on a second monitor attached to my MacBook Pro, while my MacBook Pro's display was in a non-HiDPI mode.  When I set both displays to NiDPI mode, I can see a difference between Firefox and Safari -- Firefox is more pixelated.
Attachment #631557 - Attachment is obsolete: true
Here's a screen shot of FF 13 in 800x600 HiDPI mode, "patched" (on the left) and "unpatched" (on the right).  I can't detect any difference in pixelation between the two -- both seem more pixelated than Safari.

To achieve the effect of Markus' patch, I just changed the Info.plist of one copy of FF 13.

The only difference I can detect between the two (which doesn't show up in a screenshot) is that menu text is less pixelated with Markus' patch than without.  This is presumably because the menus are actually native objects (unlike everything else in Firefox's UI).
> The only difference I can detect between the two (which doesn't show
> up in a screenshot) is that menu text is less pixelated with Markus'
> patch than without.

It's hard to tell, but the menu icons seem equally pixelated.  Which
makes sense considering	neither "version" of FF 13 has high-resolution
bitmaps in the images corresponding to the menu icons.
(In reply to comment #3)

I'm not able to reproduce what you report.  I tried with both FF 3.6.21 and 3.6.19, using this bugzilla page and the stardard 3.6.X startup page.  In both cases the fonts were just as pixelated as with later FF versions, and quite a bit more pixelated than the same pages in Safari.

Do you mean anything special (specific) by "fonts on webpages"?  Were you using any non-default settings that might have made a difference?

(It's been a long time since your comment, Andre.  But I hope you're still listening here.)
(Following up comment #19)

> The only difference I can detect between the two (which doesn't show
> up in a screenshot) is that menu text is less pixelated with Markus'
> patch than without.

This turns out to be true only of context menus.  Text in the main
menu is non-pixelated (or no more pixelated than in Safari) even
without Markus' patch.
(In reply to comment #2)

> I think we should start here by implementing
> nsChildView::GetDefaultScale() to return something other than 1.0
> when appropriate.

This doesn't work.

I tried doing this unconditionally (and also setting
layout.css.devPixelsPerPx to -1.0).  But all it does is make every
object twice as wide and twice as high, even in	a HiDPI display mode.

Moreover, fonts and graphics are just as pixelated, and are more
pixelated than they are in Safari if you zoom Safari to the same
dimensions.  (Many (most?) graphics on web pages don't look better in
Safari than in Firefox -- presumably because they don't have
sufficiently high-resolution bitmaps.  But the zebras currently on
Apple's home page (in the ad for the new MacBook with a Retina
display) do have a high enough resolution bitmap, and do look better
in Safari.)
For those who are curious:

I tried making FF use ATS fonts on all versions of OS X (by making gfxMacPlatformFontList::UseATSFontEntry() always return true).  This didn't make any difference -- our fonts are still just as pixelated in a HiDPI display mode.

(Normally we use the CGFont/CTFont APIs on OS X 10.6 and up, because the ATS fonts are so buggy.  See bug 663688.)
(In reply to Steven Michaud from comment #19)
> The only difference I can detect between the two (which doesn't show up in a
> screenshot) is that menu text is less pixelated with Markus' patch than
> without.  This is presumably because the menus are actually native objects
> (unlike everything else in Firefox's UI).

We should consider uplifting simple patches like this if the risk is judged low enough. Get some low hanging fruits released ASAP.
Comment on attachment 624738 [details] [diff] [review]
add NSPrincipalClass entry

> We should consider uplifting simple patches like this if the risk is
> judged low enough. Get some low hanging fruits released ASAP.

I agree.  Though this patch does almost nothing by itself, it's
probably a prerequisite for doing anything else.

I can't think of any way in which this patch would cause harm, so I'm
r+ing it.

Markus, are you able to land your patch, or should someone else do it
for you?
Attachment #624738 - Flags: review+
I'm not able to, please land it for me.
> I'm not able to, please land it for me.

Will do.
Comment on attachment 624738 [details] [diff] [review]
add NSPrincipalClass entry

Landed on mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c56ea8815618

Whoever lands this, please don't mark this bug fixed.  Our work has only just begun :-(
Blocking/related to bug 378927 and bug 348059?
https://hg.mozilla.org/mozilla-central/rev/c56ea8815618
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
(In reply to Steven Michaud from comment #31)
> Whoever lands this, please don't mark this bug fixed.  Our work has only
> just begun :-(
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Apologies for the incorrect resolution - testing a semi-automated tool for post-merge bug marking, so our sheriffs don't have to spend hours manually resolving 100+ bugs. It looks for [leave open] in the whiteboard, per https://wiki.mozilla.org/Inbound_Sheriff_Duty, and won't resolve such bugs.
Depends on: 763918
Just to add to comment 35, humans currently look for that whiteboard annotation too, so this would have been inadvertently closed either way.
Status: REOPENED → NEW
I'm going to put this bug aside for a while and work on other, more urgent bugs.

It'll probably be several weeks before I can get back to it.
(In reply to comment #32)

> Blocking/related to bug 378927 and bug 348059?

Not really (though all three bugs are about the same general problem).

The reason is that this bug is Mac-specific and HiDPI-specific, and fixing it probably won't require any changes to cross-platform code.  (In fact, as you can see from comment #25, plausible changes to cross-platform code may actually cause harm.)
> fixing it probably won't require any changes to cross-platform code

Aside from the need to create higher-resolution bitmaps for our browser chrome.
Fingers crossed this bug is completed before 2015.
What optimism Mil. Are all the assets as vectors online anywhere? Get the community to make hires versions. 

Mozilla are already over a year behind getting a lot of OS X features in release versions (full screen as an example).  My experience is people are dumping Firefox on the Mac because Mozilla are not progressing quick enough for the platform.
>If the “Open in Low Resolution” checkbox is selected by default for your
>app—whether >the checkbox is available (dimmed) or not—you can change the
>default by:
>
> -   Fixing all bugs related to high resolution
>
> -   Setting the NSHighResolutionCapable attribute to YES, in the Info.plist 
>     for the app, as shown in Figure 1-8. 

From:
http://developer.apple.com/library/mac/#documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/Explained/Explained.html
We're really missing out on a lot of free marketing here.

Apple community is lapping up Chrome offering retina support http://chrome.blogspot.co.uk/2012/06/chrome-and-new-shiny.html
My Retina-display MacBook Pro arrived this morning; I've made a fresh build of Firefox and can confirm that on this machine, in HiDPI mode, I see high-resolution window decorations and pop-up menus.

The rest of visible content remains low-resolution, but we aren't being forced down the whole-app pixel-doubling path. Yay!
Brion, you post some screenshots?

And what do you mean by "a fresh build".  Did you make any code changes?

Are your results with your "fresh build" better than with (say) today's mozilla-central nightly?  If so, please also post some screenshots from the nightly for comparison.

> I see high-resolution window decorations and pop-up menus

I already knew about the pop-up menus (with Markus' patch).  But in my own tests I haven't seen any improvements in "window decorations".  So please be more precise what you mean by that phrase.

Also note that in my own tests, it's our text (our fonts) that look worst in HiDPI mode.  Have you found any way to fix that?
My build is against changeset:   96792:4e3362864fbd

Behavior seems about the same as current download at http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/firefox-16.0a1.en-US.mac.dmg


Screenshot was taken with no code changes.
* window decorations (close/minimize/maximize buttons and the corners of the windows) show high-res
* AwesomeBar drop-down text shows high-res
* everything else is low-res

If I go into about:config and set "layers.acceleration.disabled" to true, then text looks beautifully high-resolution, as do the thumbnails on the new tab page. (will post screens)


I also tried making a slight change to nsCocoaWindow.GetDPI to call [aWindow backingScaleFactor] instead of [aWindow userSpaceScaleFactor] to remove a deprecation warning, but it appears not to make any effect itself (I still get high-res text if layers acceleration is disabled, with or without this change).
Currently nightly build (unmodified), with "layers.acceleration.disabled" to true via about:config.

With acceleration disabled, fonts render in very nice high resolution.
> If I go into about:config and set "layers.acceleration.disabled" to
> true, then text looks beautifully high-resolution, as do the
> thumbnails on the new tab page.

Very interesting.  Thanks for finding this out, and letting us know. 

We can't turn off layers acceleration.  But knowing that this setting
makes a difference gives us a much better idea where we'll need to
make changes.
(In reply to Steven Michaud from comment #49)
> We can't turn off layers acceleration.  But knowing that this setting
> makes a difference gives us a much better idea where we'll need to
> make changes.

Indeed, if I had access to the hardware my first attempt would be poke in LayerManagerOGL and see if we're compositing a 1:1 or 2:1 with hardware acceleration. If it's 1:1 then we likely have to tweak CoreGraphics, if not we have to tweak nsChildView widget and the layer manager.
I'll be more than happy to run any exploratory test patches you guys can devise.
By they way, the powers that be have changed their minds about how urgent they think this bug is.  So I'll get started on it next week.  With luck the same powers that be will be giving me a new Retina MBP :-)

The first thing I need to do is work more carefully through the various docs I've found.  But thanks for your suggestion, Benoit.  I'll definitely keep it in mind.
Assignee: nobody → smichaud
Hi Stevan, you might already know this, but existing (non-Retina) Mac users can enable and test HiDPI mode with these steps:

1. Download Graph Tools for Xcode (includes Quartz Debug) from Xcode > Open Developer Tool > More Developer Tools... menu in Xcode 4.3, or download it from http://developer.apple.com/downloads directly. 
2. Launch Quartz Debug.
3. Choose Window » UI Resolution and enable "HiDPI".
4. Log out and re-login to activate the change.

Aftewards, the Displays preference in System Preferences will now have '(HiDPI)' appended to their name.

As a data point, I'm currently writing this comment in "720x450 (HiDPI)" mode in a late-2008 MacBook Pro with Firefox Nightly, and indeed the layers.acceleration.disabled setting made all the difference -- Thank you Benoit!
Related bug 765914: -moz-device-pixel-ratio reports as 1.0 when running on HiDPI screen. Until that's fixed we can't target high-resolution images in stylesheets. (Firefox's chrome styles may need the same thing fixed to implement loading of appropriate assets.)
(In reply to Steven Michaud from comment #49)
> > If I go into about:config and set "layers.acceleration.disabled" to
> > true, then text looks beautifully high-resolution, as do the
> > thumbnails on the new tab page.
> 

Changing this setting in 13.0.1 on the MacBook Pro with Retina display has no effect for me. It may only affect the display when simulating HiDPI.
(In reply to Harvey Chapman from comment #55)
> > > If I go into about:config and set "layers.acceleration.disabled" to
> > > true, then text looks beautifully high-resolution, as do the
> > > thumbnails on the new tab page.
> > 
> 
> Changing this setting in 13.0.1 on the MacBook Pro with Retina display has
> no effect for me. It may only affect the display when simulating HiDPI.

That'll only work on a current nightly build (16.x) -- 13.x doesn't have the right goodies in the Info.plist and gets run in zoomed compatibility mode.
(In reply to Brion Vibber from comment #56)
> That'll only work on a current nightly build (16.x) -- 13.x doesn't have the
> right goodies in the Info.plist and gets run in zoomed compatibility mode.

Ah, confirmed. I downloaded the latest nightly and with that setting, the text looks crisp.
A fix here will probably involve returning the correct number from nsIWidget::GetDefaultScale(). By returning 2.0 we'll treat every CSS pixels as 2x2 device pixels. We will probably need to have something somewhere to ensure that window dimensions are all in device pixels and that our rendering isn't scaled up by 2 again.
(In reply to comment #58)

Probably not.

See comment #25.
That's probably because whatever already scales our output up by a factor of 2 keeps on doing so after we've added our own default scale. We need to opt out of whatever that is.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #60)
> That's probably because whatever already scales our output up by a factor of
> 2 keeps on doing so after we've added our own default scale. We need to opt
> out of whatever that is.

It's a transform that the CGContext we get in drawRect comes with. If we reverse that transform so that one user space point maps to one dev pixel again, things go right.

The problem with that is that all units on nsIWidget APIs and friends (screen, screen manager, accessible) are expected to be in dev pixels, so in Cocoa-land we'd have to multiply all our Cocoa coordinates with the backingScaleFactor to get dev pixels.
The other solution I see is to create a new fractional unit, say "screen points", that maps to the CGFloat points which the Apple docs want us to use everywhere, and use that for all screen pixel APIs. Then we'd have four coordinate units (app units, css pixels, dev pixels and screen points).
I'm not sure which approach is worse. The screen point approach looks a little better in some places, for example when the UI resolution changes dynamically (which happens for example when a window is dragged from a non-HiDPI screen to a HiDPI screen): When that happens, the window's dev pixel size changes, so it'd not only fire a resolution change event, but also a resize event, even though its size doesn't really change. With the screen point approach, the window's screen point size would stay the same and it'd only fire a resolution change event.
At first I thought there were more places that would cause problems with the dev pixel approach, for example double screen coordinates on DOM events, but those aren't a problem because those coordinates are in CSS pixels, not in dev pixels.

I'll upload the patches that I have up to now, but unfortunately I can't justify spending any more time on this in the coming four to six weeks, so maybe Steven can drive them to conclusion.
Attached patch native theme rendering changes (obsolete) — — Splinter Review 
This patch makes our native theme rendering work with HiDPI. It can be tested without running in HiDPI mode, just by setting the layout.css.devPixelsPerPx to 2.0 or by zooming normal web pages.

Cocoa renders controls in high resolution if it detects an appropriate scale transform on the context we give it. In some cases we render controls into bitmap contexts first (see the hunks which have CGBitmapContextCreate in them), so for high resolution rendering we need to double the size of these bitmap contexts and set a scale transform on them.

The rest of the patch ensures that native widgets rendered in zoomed pages are upscaled rather than extended at devpixel resolution. This is best explained with an example: If we have a multi-line text field on a webpage that is 400x300 pixels big, we'll render that with a one pixel border. If we now zoom that page to 2x, native theme rendering will render a 800x600 text field which still has a 1px border; as a result, the text field will look stretched rather than zoomed. This patch makes us render a 800x600 text field with 2px border instead, by pushing a 2x scale transform on the context and continuing with a size of 400x300 (which then gets scaled to the right size by the context).
For that to work correctly, we also need to scale the results of GetWidgetBorder and GetMinimumWidgetSize.
Attachment #635246 - Flags: review?(smichaud)
Component: General → Widget: Cocoa
Product: Firefox → Core
QA Contact: general → cocoa
Target Milestone: Firefox 16 → ---
(In reply to Markus Stange from comment #61)
> It's a transform that the CGContext we get in drawRect comes with. If we
> reverse that transform so that one user space point maps to one dev pixel
> again, things go right.

OK.

> The problem with that is that all units on nsIWidget APIs and friends
> (screen, screen manager, accessible) are expected to be in dev pixels, so in
> Cocoa-land we'd have to multiply all our Cocoa coordinates with the
> backingScaleFactor to get dev pixels.
> The other solution I see is to create a new fractional unit, say "screen
> points", that maps to the CGFloat points which the Apple docs want us to use
> everywhere, and use that for all screen pixel APIs. Then we'd have four
> coordinate units (app units, css pixels, dev pixels and screen points).
> I'm not sure which approach is worse. The screen point approach looks a
> little better in some places, for example when the UI resolution changes
> dynamically (which happens for example when a window is dragged from a
> non-HiDPI screen to a HiDPI screen): When that happens, the window's dev
> pixel size changes, so it'd not only fire a resolution change event, but
> also a resize event, even though its size doesn't really change. With the
> screen point approach, the window's screen point size would stay the same
> and it'd only fire a resolution change event.

One problem with the "screen points" approach is that layout and gfx then aren't aware of what the true device pixels are, which will cause all kinds of problems. For example, all temporary bitmaps and surfaces (such as used by the GL compositor, for example, and our blurring code) will be the wrong size and cause unnecessary pixelation. Another example, CSS borders are rounded to the nearest device pixel (minimum 1), so we'll be rounding borders unnecessarily to a multiple of 2 screen pixels. That's one case where we actually tune our layout based on the size of a device pixel, so we actually want to do a reflow when the size of a device pixel changes (which is rare anyway I hope!).

We need to tell layout the truth about the size of a device pixel. If we try to do that by keeping device pixels unchanged and fixing the above problems by passing around extra information about the true size of a device pixel, it'll be tons of work that adds a lot of unnecessary complexity to layout. I think it'd be a lot less work and a lot more maintainable to change the Mac nsIWidget implementation to work with device pixels that match actual screen pixels.

If we do that, we'd still need similar changes to the native theme implementation so that the widgets are upscaled correctly.
Here's my overkill experiment to introduce screen points. Screen points need to be in fractional units, so I've used gfxFloat (as "double" in idl) and gfxRects, but that's probably a bad idea since this type already has an existing meaning ("fractional dev pixel for rendering" or something like that), and it's prefixed and obsoleted by prefix-free types, etc.
The reason I did this was that a new type causes the compiler to point me to all the conversion points that need to be updated. And the rabbit hole goes pretty deep...
This patch is really incomplete and up to now only compiles for 64bit mac builds.
Here's a summary of the juicy portions:
 - nsDeviceContext asks nsIWidget for its DevPixelsPerScreenPoint and bases all conversions on that.
 - all screen pixels APIs now use screen points (except some DOM APIs that use int css pixels, and nsIWidget invalidation and plugin clipping is still dev pixels)
 - system font size conversion in nsRuleNode.cpp expected nsILookAndFeel font sizes to be in dev pixels; changed that to screen points
    ^ this is also the reason that text is too small when changing the devPixelsPerPx pref
 - added ClientSurfaceSize to nsIWidget so that OpenGL viewport size is correct
 - did the context scale inversion in drawRect (see beginning of comment 61)

There's probably more but I can't think of anything atm.

I haven't tested plugins with this patch, and selection drag image rendering is broken, and there are a few XXX comments that need to be addressed, but apart from that it works pretty well.
Attachment #635247 - Flags: feedback?(roc)
Handling dynamic resolution changes is surprisingly easy.

Anyway, that's all I have and my time for this is up, so good luck Steven :)
That patch is an incredible effort but do you really think it's simpler than adjusting the Mac implementations of APIs that work with device pixels?
I haven't had any chance to work on this in the last week or so.  I've been clearing the decks of more urgent bugs (notably those having to do with support for the Lion fullscreen mode, which will appear for the first time in a release in FF 14).

But now I'm done.

The first thing I'll do is read carefully through Apple's docs, then through Markus' patches.  That should give me some idea how to proceed.
For those who are curious, I've found a way to improve Firefox's appearance in HiDPI mode while making no changes at all in cross-platform code.  (The patch needs a bit more work, so I'll post it on Monday, together with a tryserver build.)

Some things look just as good as in Safari -- for example the zebra graphic and the text in the Retina MBP ad at http://www.apple.com/.  Other text (for example in the location bar) looks better than in current nightlies, but not quite as good as in Safari.

All my tests have been with OpenGL on (with layers.acceleration.disabled off, as it is by default).

By the way, I did confirm that Firefox nightlies look just as good as Safari when you turn OpenGL off.  At least all text looks just as good.
(Following up comment #68)

Where, in my new build, the text looks better than in FF nightlies but less good than in Safari, it appears that it's the same resolution as in FF nightlies (half that of Safari) but has been antialiased (as it isn't in Firefox nightlies).

Why this is I don't know.  I'll attempt to find out more on Monday.
Here's the patch I promised on Friday.

On retesting, I decided that nothing in it looks as good as in Safari
-- even the zebras graphic and the text in the Retina MBP ad at
http://www.apple.com/.  But everything looks better in it than in a
current mozilla-central nightly.  (With	OpenGL on, that	is.)

I now think that *everything* (including the zebras graphic) is
somehow "antialiased".

I've started a tryserver build, which should be available in a few
hours.  I'm particularly anxious to hear from testers who have the new
Retina MBP -- I still don't have mine.

Clearly	we've got more work to do.  But	at least this patch is a
start.

(Followed up in next comment).
(Following up comment #70)

Apple's HiDPI mode is a strange beast:  Apple *doesn't* want apps to
just start behaving as if the screen contained four times as many
pixels as previously (their number doubled horizontally and
vertically) -- this would just make every UI object four times as
small (twice horizontally and twice vertically).  They *do* want each
UI object to (potentially) have four times the detail as previously.

Given this, I don't think we should change the resolution at which we
lay out Firefox, or even necessarily at which we draw Firefox.  If we
find (as we do when not using OpenGL) that Apple automatically handles
the (potentially) increased resolution correctly, I think we should
let them do it.

We should only intervene when (as in the case of OpenGL) Apple's
automatic handling (its pixel doubling) *doesn't* work correctly.

My patch from comment #70 somehow adds "antialiasing" to Apple's pixel
doubling.  This does improve our appearance, but is only a stopgap.

Over the next few days I'll be looking through OpenGL code for places
where I can make it optionally use more pixels than "normal". 

Benoit, you're probably the one who knows our OpenGL code best.  Do
you have any suggestions how I should go about this?
Here's a tryserver build made with my patch from comment #70:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-17fb66eca567/try-macosx64/firefox-16.0a1.en-US.mac.dmg

If you're able to test in a HiDPI mode please try it out -- particularly if you have a Retina MacBook Pro.

I'm particularly interested in how it compares to Safari and to current mozilla-central nightlies.  Since the Retina MBP ad at http://www.apple.com/ (particularly the zebras graphic) was surely intended to show off the new Retina displays, that's a good place to start.
Wikipedia homepage in Safari (left) & Firefox try build (right) on Retina MacBook Pro

With graphics acceleration enabled.

It's definitely filtering/antialiasing the final output (which it didn't used to), but it looks like it's still scaled up from low resolution.
Also a warning -- http://www.apple.com/ doesn't load the high-resolution zebra image in Firefox; I think it's explicitly checking for Safari.

You can grab the image direct however, and with it zoomed out you can easily see the difference between full resolution and pixel-doubling.

http://images.apple.com/home/images/macbookpro_hero_2x.jpg

This screenie is with acceleration disabled -- so you can see it looks great in the side-by-side at full res.
And here with acceleration enabled on the try build -- image renders blurry, definitely getting squashed to low resolution somewhere in the pipeline.
Looking at the JavaScript on http://www.apple.com/ it's checking for window.devicePixelRatio to determine whether to load high-res images. Bug 564815 requests implementation of that interface.
Using the test build in comment 72, top down: no accel, accel, safari.
(In reply to Steven Michaud from comment #71)
> Apple's HiDPI mode is a strange beast:  Apple *doesn't* want apps to
> just start behaving as if the screen contained four times as many
> pixels as previously (their number doubled horizontally and
> vertically) -- this would just make every UI object four times as
> small (twice horizontally and twice vertically).  They *do* want each
> UI object to (potentially) have four times the detail as previously.

Of course.

> Given this, I don't think we should change the resolution at which we
> lay out Firefox, or even necessarily at which we draw Firefox.

Yes we should. Layout knows about the difference between "CSS pixels" (resolution independent) and "device pixels" (resolution dependent). We take advantage of knowledge of device pixels to do resolution-dependent work such as sizing internal buffers and other things I described in comment #63 --- e.g. rounding border widths to device pixels and snapping text baselines and underlines to device pixels. We need to exploit that support instead of working around it, and that means we have to change the ratio of device pixels to CSS pixels so that device pixels match screen pixels. I will r- any approach that doesn't do that.
> I will r- any approach that doesn't do that.

Even if your approach doesn't work with Apple's HiDPI mode?

I'll keep an open mind about what you say.  It may very well turn out to be the right way to do things.  But I ask that you also keep an open mind.
(In reply to Steven Michaud from comment #79)
> > I will r- any approach that doesn't do that.
> 
> Even if your approach doesn't work with Apple's HiDPI mode?

Unless HiDPI mode is very strange and won't let us get our choice of pixels on the screen, there will be a way to make it work. So I suggest you at least try my approach first.
(In reply to Brion Vibber from comment #74)

The 2x (retina) Zebra asset is called by safari via a window.devicePixelRatio check.  If a value of 2 is returned, http://images.apple.com/home/images/macbookpro_hero_2x.jpg is used.  Otherwise, http://images.apple.com/home/images/macbookpro_hero.jpg is used.  This is common logic on most of apple's site.  (For this page the logic is in http://images.apple.com/global/ac_retina/ac_retina.js )

Apple recommends you check display resolution via window.devicePixelRatio in js or -webkit-min-device-pixel-ratio.  They have also proposed the image-set function in css level 4.
(In reply to Mike Deerkoski from comment #81)

Note related bug: -moz-device-pixel-ratio misreported as 1.0 instead of 2.0 on Mac HiDPI display
http://bugzilla.mozilla.org/show_bug.cgi?id=765914

-device-pixel-ratio is being returned as 1, on an apple HiDPI display, while safari returns 2.
(In reply to Mike Deerkoski from comment #82)
> Note related bug: -moz-device-pixel-ratio misreported as 1.0 instead of 2.0
> on Mac HiDPI display
> http://bugzilla.mozilla.org/show_bug.cgi?id=765914

Right, that will be fixed if we take approach in comment #78.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #66)
> That patch is an incredible effort but do you really think it's simpler than
> adjusting the Mac implementations of APIs that work with device pixels?

It's definitely not the simpler patch, but it might save us some headaches in the long run. I've thought about this a little more, and while I haven't found a really convincing argument, here are the ones that I've come up with:
 - When somebody adds a new widget API and forgets about OS X HiDPI mode, we're more likely to fail gracefully. For example, say we add a canvas-as-dragimage feature (which we already have, but assume we didn't), I could imagine that with the device pixel approach we'd map one canvas pixel to one device pixel by default, which would make the image too small under HiDPI.
 - Having the screen point unit makes people less like to forget about HiDPI mode when adding widget APIs. (But it would also make them aware of it even when their work doesn't have any implications on HiDPI, which would be an unnecessary cognitive load, which is bad.)
 - The total number of unit conversion calls might be lower with the screen point approach. Sure, the initial patch needs to change many conversion calls, but it doesn't add many additional conversions, it only changes which units to convert from/to. (Again, I'm cheating a little here since I haven't looked at the implications for widget API implementations on other platforms; if those have to introduce casts to integer everywhere, this argument is nullified.)
 - Debugging widget API calls under OS X HiDPI mode is more confusing when everything is in dev pixels because one needs to halve all numbers when comparing them to the ones we get from Cocoa.
 - I see some merit to the advice that the HiDPI documentation gives, that we should only be aware of dev pixels when we must, and treat window / screen dimensions independently from them. But the practical implications of this are probably mostly in backward- / forward-compatibility, which we have already anyway because we use appunits / css pixels internally.

In any case, the decision for the right approach is up to you, roc :)
(In reply to Markus Stange from comment #84)
>  - When somebody adds a new widget API and forgets about OS X HiDPI mode,
> we're more likely to fail gracefully. For example, say we add a
> canvas-as-dragimage feature (which we already have, but assume we didn't), I
> could imagine that with the device pixel approach we'd map one canvas pixel
> to one device pixel by default, which would make the image too small under
> HiDPI.

This isn't a new issue since we already have conversions between CSS pixels (which canvas sizes are specified in) and device pixels, which are not 1:1 when a Web page is zoomed for example.

>  - Having the screen point unit makes people less like to forget about HiDPI
> mode when adding widget APIs. (But it would also make them aware of it even
> when their work doesn't have any implications on HiDPI, which would be an
> unnecessary cognitive load, which is bad.)

True I guess, although nsIntRect/nsIntPoint is already consistently used for device pixels throughout layout. The only thing we'd be changing here is that Cocoa pixels != device pixels.

>  - The total number of unit conversion calls might be lower with the screen
> point approach. Sure, the initial patch needs to change many conversion
> calls, but it doesn't add many additional conversions, it only changes which
> units to convert from/to. (Again, I'm cheating a little here since I haven't
> looked at the implications for widget API implementations on other
> platforms; if those have to introduce casts to integer everywhere, this
> argument is nullified.)

Maybe but we don't call in and out of widget all that often so I don't think this matters much.

>  - Debugging widget API calls under OS X HiDPI mode is more confusing when
> everything is in dev pixels because one needs to halve all numbers when
> comparing them to the ones we get from Cocoa.

True.

>  - I see some merit to the advice that the HiDPI documentation gives, that
> we should only be aware of dev pixels when we must, and treat window /
> screen dimensions independently from them. But the practical implications of
> this are probably mostly in backward- / forward-compatibility, which we have
> already anyway because we use appunits / css pixels internally.

That might be more true for regular apps that don't already have an implementation specialized to work with knowledge of device pixels.

Even if we took all those points as given, it wouldn't change the fact that we have a ton of code in layout that depends on knowing what a device pixel is, and we need to tell it the truth.
An alternative that would work is to redefine all the coordinates in widget interfaces to be CSS pixels, make them the same as Cocoa pixels on Mac, and introduce conversions from CSS pixels to device pixels (or vice versa) everywhere we use widget coordinates in Gecko. We would still use GetDefaultScale() to compute the default device pixel to CSS pixel ratio (although it might make sense to rename it).

We currently don't have any platforms using GetDefaultScale() so we could do this only changing cross-platform code; widget implementations would not have to be changed.

Whether this is a good idea partly depends on whether the platforms where we introduce high-DPI modes in the future use device pixels or some resolution-independent analogue of CSS pixels in their native widget APIs. E.g. Mac uses resolution-independent APIs in Cocoa, but if Windows and mobile use device pixels, then we'd have to sprinkle a lot of conversions in their widget implementations, and we'd sometimes be double-converting from the device pixels to CSS pixels and then back to device pixels in Gecko.
Ooh, I have another, very practical, argument:
 - When a window overlaps between a HiDPI and a non-HiDPI screen, window coordinates in device pixels are ambiguous. For example, if the left half of the window is on a HiDPI screen and the right half on a non-HiDPI screen, the window height on the left screen will be twice the number of device pixels compared to on the right screen. (Cocoa will change the window's backingScaleFactor dynamically depending on which screen the window occupies more area on, and the window manager will apply a scale when rendering the window on the other screen.) Gecko somewhere has a FindScreenForWindow method IIRC, and this method will have a hard time finding the right screen if it doesn't know which screen the device pixel coordinates are relative to.

> Even if we took all those points as given, it wouldn't change the fact that
> we have a ton of code in layout that depends on knowing what a device pixel
> is, and we need to tell it the truth.

In the screen point patch pretty much all of this works correctly, I think. There are methods that convert between screen points and device pixels and I think I got most of the callers right.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #86)
I'll think about this a little.
(In reply to Markus Stange from comment #87)
>  - When a window overlaps between a HiDPI and a non-HiDPI screen, window
> coordinates in device pixels are ambiguous. For example, if the left half of
> the window is on a HiDPI screen and the right half on a non-HiDPI screen,
> the window height on the left screen will be twice the number of device
> pixels compared to on the right screen. (Cocoa will change the window's
> backingScaleFactor dynamically depending on which screen the window occupies
> more area on, and the window manager will apply a scale when rendering the
> window on the other screen.) Gecko somewhere has a FindScreenForWindow
> method IIRC, and this method will have a hard time finding the right screen
> if it doesn't know which screen the device pixel coordinates are relative to.

I don't see the problem there as long as we always stay in sync with Cocoa about what screen the window is on and what its backingScaleFactor is. I'm pretty sure we'll need to do that whatever solution we pick.

> In the screen point patch pretty much all of this works correctly, I think.
> There are methods that convert between screen points and device pixels and I
> think I got most of the callers right.

Do we definitely need to support fractional screen points? If so, then maybe if we replace "screen points" with "CSS pixels" then your patch is the right thing.

However I would like to know about high-DPI mode on other platforms. As far as I can tell, on Windows, when we declare ourselves to be "DPI aware" then all Windows APIs will still be working in device pixels. So if we take your patch then all the problems you're trying to avoid on Mac will crop up on Windows, but in reverse.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #88)
> I don't see the problem there as long as we always stay in sync with Cocoa
> about what screen the window is on and what its backingScaleFactor is. I'm
> pretty sure we'll need to do that whatever solution we pick.

Hmm, possible. I'd need to look at the code.

> > In the screen point patch pretty much all of this works correctly, I think.
> > There are methods that convert between screen points and device pixels and I
> > think I got most of the callers right.
> 
> Do we definitely need to support fractional screen points?

Not yet, as far as I can tell. Mouse coordinates and window dimensions were always integer screen points in my tests.
I chose gfxFloat / gfxRect because using a different C++ type makes it clear that other coordinate types need an explicit conversion, and because Cocoa uses CGFloat which is a double, too.

> If so, then maybe
> if we replace "screen points" with "CSS pixels" then your patch is the right
> thing.
> 
> However I would like to know about high-DPI mode on other platforms. As far
> as I can tell, on Windows, when we declare ourselves to be "DPI aware" then
> all Windows APIs will still be working in device pixels.

OK.

> So if we take your
> patch then all the problems you're trying to avoid on Mac will crop up on
> Windows, but in reverse.

It would happen if we widget APIs were in CSS pixels, like you've suggested. 
But it doesn't happen with my patch because I'm not using GetDefaultScale. Instead, I've added nsIWidget::DevPixelsPerScreenPoint and left GetDefaultScale at 1.0 just so that other platforms can keep a 1:1 screen point per dev pixel ratio and still use the GetDefaultScale magnification capability. In my patch, GetDefaultScale gives the number of screen points per CSS pixel, not the number of device pixels per CSS pixel.

You can see this in the change to nsDeviceContext::SetDPI: both screenPointsPerCSSPixel and mDevPixelsPerScreenPoint come from the widget (or the pref), and they're multiplied to get devPixelsPerCSSPixel which then gives mAppUnitsPerDevNotScaledPixel.
Roc, I see your point about being reluctant to lie to layout and gfx about device pixels.  And surely we want to eventually not have to lie, on any platform that supports HiDPI.  But it sounds like this will be a lot of work, and will take us a while (especially to iron out the kinks that this work is likely to introduce).

It'd help if we had quicker ways to partially implement support for Apple's HiDPI mode -- ones that could be backed out once we'd figured out how to implement full support correctly.  That's what I'm going to be working on ... at least for a few days, and unless I find that it doesn't pan out.

Right now, as best I can tell things work just fine on the Mac in HiDPI mode when we don't use OpenGL, even though we're (in effect) lying to layout and gfx about device pixels.  (There may be problems, but they don't appear to be gross ones.  Do please suggest, though, where we might look to find them.)

It's hard to say *why* we don't see gross problems.  But it may help that all of our bitmapped "assets" (window chrome, icons and so forth) are still at their old, low resolutions.

It's hard to tell, but I think our bitmapped assets look the same in HiDPI mode as they do in non-HiDPI mode, even when we don't turn on OpenGL.  But it's striking how much better our fonts look in HiDPI mode (if we don't turn on OpenGL).

I'm hoping that if we don't (for the time being) increase the resolution of our bitmapped assets, layout and gfx won't be too badly effected by being lied to about device pixels.  I'm also hoping that I can hack out a way to make our fonts look as good (in HiDPI mode) with OpenGL on as they do with OpenGL off, without having to lie too atrociously to layout and gfx.
(In reply to Steven Michaud from comment #90)
> Roc, I see your point about being reluctant to lie to layout and gfx about
> device pixels.  And surely we want to eventually not have to lie, on any
> platform that supports HiDPI.  But it sounds like this will be a lot of
> work, and will take us a while (especially to iron out the kinks that this
> work is likely to introduce).

I think we should try to do things right now instead of piling up more technical debt.

> Right now, as best I can tell things work just fine on the Mac in HiDPI mode
> when we don't use OpenGL, even though we're (in effect) lying to layout and
> gfx about device pixels.  (There may be problems, but they don't appear to
> be gross ones.  Do please suggest, though, where we might look to find them.)

box-shadow, text-shadow, animated CSS opacity on an element containing text, CSS filters and SVG masks on elements containing text.

I don't expect those things to look worse. They just won't look better where they would look better if we do things right.
(In reply to Markus Stange from comment #89)
> Not yet, as far as I can tell. Mouse coordinates and window dimensions were
> always integer screen points in my tests.
> I chose gfxFloat / gfxRect because using a different C++ type makes it clear
> that other coordinate types need an explicit conversion, and because Cocoa
> uses CGFloat which is a double, too.

We can easily create new rect and point types that use integer coordinates but are different from nsIntRect and nsRect. I think we should do that since otherwise we're introducing a whole lot of floating point conversions that non-Mac platforms don't need.

> > So if we take your
> > patch then all the problems you're trying to avoid on Mac will crop up on
> > Windows, but in reverse.
> 
> It would happen if we widget APIs were in CSS pixels, like you've suggested. 
> But it doesn't happen with my patch because I'm not using GetDefaultScale.
> Instead, I've added nsIWidget::DevPixelsPerScreenPoint and left
> GetDefaultScale at 1.0 just so that other platforms can keep a 1:1 screen
> point per dev pixel ratio and still use the GetDefaultScale magnification
> capability. In my patch, GetDefaultScale gives the number of screen points
> per CSS pixel, not the number of device pixels per CSS pixel.
> 
> You can see this in the change to nsDeviceContext::SetDPI: both
> screenPointsPerCSSPixel and mDevPixelsPerScreenPoint come from the widget
> (or the pref), and they're multiplied to get devPixelsPerCSSPixel which then
> gives mAppUnitsPerDevNotScaledPixel.

Ah, OK. So on Mac screen points == CSS pixels, but on Windows screen points == device pixels. That would work.

However, I'm still not happy about the maintenance burden of adding yet another coordinate system, with Gecko converting from screen points to and from CSS pixels and device pixels :-( ... where on any given platform, some set of those conversions are always doing nothing.

I still lean towards avoiding that by making all widget APIs use device pixels and adjusting Cocoa coordinates to device pixels in the Mac widget code.
Nightly 16.0a1 (2012-07-01) has regressed -- I'm no longer getting sharp high-resolution text either with acceleration off or on.

I can confirm that going back to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-06-30-03-05-32-mozilla-central/firefox-16.0a1.en-US.mac.dmg shows high-resolution text with acceleration off.
Interesting.  I'll try to figure out what happened.
(In reply to comment #93)

I've confirmed what you report.

Text doesn't look as bad (in a HiDPI mode) as it does when using OpenGL, but doesn't look as high resolution as it used to.

Here's the regression range in terms of patches landed:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f08d285b63b0&tochange=d9d61d199b11

I suspect the culprit is one of the patches for bug 539356.  But I'll need to use hg bisect to confirm that (and find out which one).
The "culprit" is

http://hg.mozilla.org/mozilla-central/rev/bd0a91621ea9
Matt Woodrow <mwoodrow@mozilla.com>
Bug 539356 - Part 8b - Move painting of retained layers to the view manager flush, and only composite on the paint event. r=roc

Which I can't (yet) make any sense of.  I'll need to dig deeper to find out.
The patches in that bug make all our repainting go through temporary buffers in ThebesLayers. Those buffers are sized based on device pixels which is why they don't get any HiDPI treatment. That change is not only needed for bug 539356 but also to enable off-main-thread-compositing when we're not using GL --- basically we're making non-GL behave more like GL.

So I wouldn't bother digging into that issue any further. That code is working as designed. Everything will be fine once we set up device pixels correctly for HiDPI.
(In reply to comment #97)

I've found that the following patch (to current code) restores previous behavior (in a HiDPI mode when not using OpenGL):

--- a/layout/base/nsPresShell.cpp       Mon Jul 02 14:05:17 2012 +0200
+++ b/layout/base/nsPresShell.cpp       Tue Jul 03 09:46:25 2012 -0500
@@ -5275,9 +5275,9 @@
         return;
       }
       layerManager->BeginTransaction();
-      if (layerManager->EndEmptyTransaction()) {
-        return;
-      }
+      //if (layerManager->EndEmptyTransaction()) {
+      //  return;
+      //}
       NS_WARNING("Must complete empty transaction when compositing!");
     } else {
       layerManager->BeginTransaction();
diff -r 155f67c2c578 view/src/nsViewManager.cpp
--- a/view/src/nsViewManager.cpp        Mon Jul 02 14:05:17 2012 +0200
+++ b/view/src/nsViewManager.cpp        Tue Jul 03 09:46:25 2012 -0500
@@ -395,7 +395,8 @@
         printf("---- PAINT START ----PresShell(%p), nsView(%p), nsIWidget(%p)\n", mPresShell, aView, widget);
 #endif
         nsAutoScriptBlocker scriptBlocker;
-        mPresShell->Paint(aView, nsRegion(), nsIPresShell::PaintType_NoComposite, false);
+        //mPresShell->Paint(aView, nsRegion(), nsIPresShell::PaintType_NoComposite, false);
+        mPresShell->Paint(aView, nsRegion(), nsIPresShell::PaintType_Composite, false);
 #ifdef DEBUG_INVALIDATIONS
         printf("---- PAINT END ----\n");
 #endif

No, I'm not suggesting we do this.

I think it means that, as of the patches for bug 539356, we're no longer ever drawing directly to the screen, and that the only time we were doing so previously was on an "empty transaction".

For this and other reasons (more about them in the next comment), I'm giving up my attempt to find a quick fix for this bug (aside from the patch I've already posted).

As things now stand, no amelioration of this bug (aside from my patch from comment #70) is possible short of a full and proper fix.  So it'll take much longer to do anything about this bug (aside from my patch) than I'd originally hoped.
Every call we make to draw text goes through CGContextShowGlyphsWithAdvances(), called from _cairo_quartz_surface_show_glyphs_cg():

http://hg.mozilla.org/mozilla-central/annotate/f38d6df93cad/gfx/cairo/cairo/src/cairo-quartz-surface.c#l2678
http://hg.mozilla.org/mozilla-central/annotate/f38d6df93cad/gfx/cairo/cairo/src/cairo-quartz-surface.c#l2825

After discovering this, I spent several days last week digging around in cairo code looking for a way to up the resolution at which we draw glyphs.  I didn't find any.

My most promising idea was to double the size of all the bitmaps we create in cairo-quartz-surface.c and of their (undocumented) "resolution" (accessible using the undocumented CGFloat CGContextGetResolution(CGContextRef)).  This made no difference that I could see.
Here's my patch from comment #70, updated to current trunk code.

This patch doesn't do much.  But it *does* improve the appearance of Firefox in HiDPI mode when OpenGL is on (as it is by default).  And it's our only viable candidate for a quick "fix".

Here's a tryserver build made from it:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-3dca4d51c4eb/try-macosx64/firefox-16.0a1.en-US.mac.dmg

This is nothing new -- it hasn't changed from my previous patch.  So don't comment on it unless you have something to say that wasn't already said about my patch from comment #70 (and its tryserver build from comment #72).
Attachment #636438 - Attachment is obsolete: true
Now that (aside from my "antialiasing" patch) I've given up trying to find a quick fix for this bug, I'm going to spend some time trying to get Cocoa widgets to use device pixels.

I won't be able to work on this full time (I've got to keep up with topcrashers and the like).  But I hope to be able to spend a few days on it next week.
This is a screenshot of some text in Firefox 13. The zoom level in Firefox is default. The text does not improve when zoomed up or down. 

The only way to make the text look pretty that I've found is to use a third party utility to set the rMBP resolution to the native 2880x1800 which makes everything tiny but then Firefox fonts are pretty and if I zoom them up with Firefox zoom controls they become legible in size and stay pretty.
I guess that's MacOS just pixel-doubling our window and that looking bad due to subpixel text basically not working in that scenario.
Asa, do test with my tryserver build.  The text in it looks quite a bit better than in FF 13, though not so good as in Safari.
Steven - tried your build on my retina macbook and it just looks blurred compared to FF13 instead of pixelated, but still nowhere near as good as Safari or Chrome Dev.
Just a brief status update... I've taken Markus's patches here and am working on updating them for current trunk; the result seems to be working pretty well (in very limited pseudo-HiDPI testing), modulo plugins (untested) and drag images (definitely broken). I'm hoping to have a tryserver build up later today so that people can try it on a real rMBP.

In parallel, I'm also trying to pull together a less-invasive approach based on Roc's suggestions here; that's significantly further from being usable at this point (I have some lovely hi-res rendering, but only filling a quarter of the window; and lots of the UI is not properly scaled). No ETA on this yet - there are far too many coordinates and rectangles still in flight....
Tryserver build based on mstange's screenPoints patches is available at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-fc19680644a8/. This certainly won't pass all tests, etc., but it should at least be usable (I hope).
I have installed Jonathan's try build (OSX 64).

The trybuild is looking much better than FF13 or FF15 (Aurora). I am having trouble distinguishing it from Safari on sites like Wikipedia.  Apple's site still shows pixelation due to using images for text.

I have taken screenshots of the browsers on Apple.com/Wikipedia.

Apple.com:
https://dl.dropbox.com/u/1736241/Screenshots/n1.png Safari
https://dl.dropbox.com/u/1736241/Screenshots/n2.png FF15 Aurora
https://dl.dropbox.com/u/1736241/Screenshots/n3.png FF16 TryBuild (Jonathan)

Wikipedia.org:
https://dl.dropbox.com/u/1736241/Screenshots/n4.png Safari
https://dl.dropbox.com/u/1736241/Screenshots/n5.png FF15 Aurora
https://dl.dropbox.com/u/1736241/Screenshots/n6.png FF16 TryBuild (Jonathan)

The Apple page is using HiDPI images that we aren't requesting yet due to device pixel ratio not equaling 2 on the HiDPI screen.

Wikipedia uses regular text everywhere and is looking fantastic on the TryBuild.
Just got my Retina machine, and indeed :jfkthame's Try build looks very good. <select> menus are fairly messed up, not sure if that's expected.
I wasn't aware of <select> being messed up, so thanks for mentioning it. Looks like the menu itself displays fine, it's just displayed in the wrong place, yes? That should be a relatively simple fix, I hope.
It's displayed in the wrong place, and making selections with the mouse is... odd.

Visually, the highlight follows the mouse, but the selection that gets made is different, possibly what would be under the mouse pointer if the menu were drawn in the correct location?

Making selections with the keyboard works fine.
Jonathan, you build looks much much better on my retina MBP. Worth noting - the address bar doesn't seem to reflect the current URL (just the last one input) and the back/forward buttons don't work...
I does look much better on my rMBP too.
I don't have address bar issues but select elements displays wrong as Jonathan said.

Here's an example:
https://dl.dropbox.com/u/5262061/ff-nightly-select-retina.png
I can reliably crash this build by dragging the horizontal scroll bars in the screenshots frame at http://itunes.apple.com/app/dashcommand-obd-ii-gauge-dashboards/id321293183?mt=8
There's another tryserver build at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-7b69fc1a2452/. This one should handle <select> menus better, and no longer has scrambled drag images for text selections. Plugins (e.g. Flash) aren't fully working yet.
Assignee: smichaud → jfkthame
(In reply to Jonathan Kew (:jfkthame) from comment #116)
> There's another tryserver build at
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-
> 7b69fc1a2452/. This one should handle <select> menus better, and no longer
> has scrambled drag images for text selections. Plugins (e.g. Flash) aren't
> fully working yet.

Looks good so far on my Retina MBP; I'll try using this for a couple days and see if anything explodes. :) I had one machine crash when first launching, but haven't reproduced it again yet.

Confirmed the <select> problem is resolved.

Switching between HiDPI and non-HiDPI seems to work fine (using SwitchResX); I'm at a conference so can't test external monitors / mixed densities just yet.
Text looks much better on my rMBP. The UI text is also HiDPI, although the UI elements aren't, e.g. tabs and buttons.

Scrolling is slower than Safari, but I don't know if that is the fault of this bug.

-webkit-min-device-pixel-ratio is unsupported obviously, since it is a webkit prefix. Is there a -moz-min-device-pixel-ratio yet? If so, I will update my website.
(In reply to will from comment #118)
> -webkit-min-device-pixel-ratio is unsupported obviously, since it is a
> webkit prefix. Is there a -moz-min-device-pixel-ratio yet? If so, I will
> update my website.

It's moz--min-device-pixel-ratio... I have an example on bug 765914 which seems to show it working as expected with this build. Yay!
Or min--moz-device-pixel-ratio rather.
Thanks! High Def images work on the build with the min--moz-device-pixel-ratio css media query. I have updated my blog accordingly...

Why is it min--moz-device-pixel-ratio rather than -moz-min-device-pixel-ratio? That seems weird.

http://www.manystrands.com/
Selects work, and I can't crash the browser as described in comment 115, but two-finger scrolling doesn't work in that frame. (may be unrelated)

Something I've noticed in both builds:
The first time after launch, if you download a file, the door-hanger showing download status will slowly shrink until it needs scroll bars. I tested with the regular nightly and did not see this behavior. (Though, the text in the door-hanger is rendered at HiDPI in the regular nightly, even though nothing else is)
(In reply to Zandr Milewski [:zandr] from comment #122)
> Selects work, and I can't crash the browser as described in comment 115, but
> two-finger scrolling doesn't work in that frame. (may be unrelated)

I think that's unrelated - I'm finding that frame quite unresponsive to two-finger scroll in a standard Nightly build as well. I can kind of "nudge" it a little bit at a time, but I can't get a proper scrolling motion to work.

> Something I've noticed in both builds:
> The first time after launch, if you download a file, the door-hanger showing
> download status will slowly shrink until it needs scroll bars.

That's....weird!
Another minor glitch I'm seeing in HiDPI mode with these patches is that the status-bar overlay thing (not sure what it's officially called) that shows link targets, etc,  in the bottom corner of the window does not skip away from the mouse to the opposite corner.
I've tried the build from comment 116, and it runs very smoothly. Text is displayed in high-resolution as expected, the Flash plugin worked perfectly for me. Minor (and probably already known) issues are: lack of high-res Firefox artwork (icons and the like).

The two real issues I observed are:
  1. high-res images in webpages like http://www.apple.com/ are not picked up
  2. The Web Developer "Inspect" mode highlights rectangles at totally wrong positions (see attached screenshot for an example). Seems the unit conversion is not done properly there.
Also of note: using the HiDPI build, the favicon doesn't appear next to the URL as it usually does. It does appear in the tab, however.
Using comment's 116 [patched build of Firefox](https://bugzilla.mozilla.org/show_bug.cgi?id=674373#c116) which handles HiDPI rendering on the new MacBook Pro with Retina display, I tried the viewer example at http://mozilla.github.com/pdf.js/web/viewer.html

The rendering of pdf.js on this HiDPI build is incorrect, with the text being rendered all blurry. The attached screenshot compares FF's rendering (top) to Safari's crystal-clear text on the same page (bottom).
Adding to comment 128, all canvas operations are performed in low resolution mode (and thus give blurry rendering). See the attached screenshot of this page: https://developer.mozilla.org/samples/canvas-tutorial/2_5_canvas_quadraticcurveto.html where patched FF is on the left, and Safari on the right.
After more testing, I continue with my list of things that appear in low resolution in the high-res patched nightly: WebGL also has this issue, which can be seen on the screenshot attached of the demo at https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/demos/google/particles/index.html

On the screen capture, compare patched FF's low-res WebGL on the left, to Chrome beta's high-res WebGL on the right.
Thanks FX. Those last three issues should be filed as separate bugs since they require extra work to be made DPI-aware.
The tryserver build is better than the original patch in that it works with hardware acceleration on. However, there's a few quirks, namely with plugins. Some don't accept input at all, others very inconsistently. Youtube videos also have their chrome rendered in HiDPI mode, making the controls very hard to access.

Cheers,
Using the latest tryserver build (found in the directory tree mentioned above).. While the text looks good on a Retina display, when you move the window onto a screen that's not Retina capable - the icons get messed up on the URL line (i.e back, forward, home, downloads, etc.) They appear with a box around them.

I attached a screen capture to look at (sorry for the poor quality as I had to convert from a video since the window has to be in focus to see the issue and window capture wouldn't allow me to do that.)

Finally, video controls (such as Youtube) don't get rendered properly (all in in HiDPI mode.)
Which is the latest tryserver build that works. Many of the links in past comments doesn't work any more
Using the last tryserver build...

Dragging a tab from a window on a HiDPI screen (internal retina MBP) to a non-HiDPI screen (Dell U2410) creates as new window as expected, but draws all the text at 2x. UI widgets appear to be rendered in the right place, but associated text is a long way away. 

Curiously, *taking a screenshot* caused all the text to re-render in the right place, but still at 2x size. Said screenshot attached.

cmd-R fixes it.
There's a newer tryserver build at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-efaf94ebbaaf/, based on a simplified version of Markus's screenPoints patch where "screen points" are integer rather than floating-point coordinates.

This also fixes a couple more things that weren't working properly, including the status panel mouse-avoidance behavior. Many plugins will still have problems, and I haven't looked into the tab-dragging issue that was just reported above.
Posting current WIP, in case anyone wants to play with it... this is the basis of the tryserver build above. Known to still have plugin problems and a few unit test failures.
(In reply to Jonathan Kew (:jfkthame) from comment #138)
> There's a newer tryserver build

Going back over the things I listed in my earlier comments as not working properly, the status of this new build is:

1. Inspect feature is now working okay
2. HiDPI images are still not picked up on pages like www.apple.com
3. Favicon still does not appear next to URL
4. WebGL still low res
5. canvas still low res
(In reply to FX from comment #140)
> (In reply to Jonathan Kew (:jfkthame) from comment #138)
> > There's a newer tryserver build
> 
> Going back over the things I listed in my earlier comments as not working
> properly, the status of this new build is:
> 
> 1. Inspect feature is now working okay

Great, thanks for testing.

> 2. HiDPI images are still not picked up on pages like www.apple.com

Judging by the testcase in bug 765914, this should work if the site uses the -moz-prefixed version of device-pixel-ratio; presumably the Apple site is only using the -webkit-prefixed version, and therefore serves us low-res images.

> 3. Favicon still does not appear next to URL

I'll try to look into this.

> 4. WebGL still low res
> 5. canvas still low res

These issues should be handled as separate followup bugs, I think.
The favicon was dropped from the address bar in nightlies a few months ago and is now like that in Firefox 14:
https://msujaws.wordpress.com/2012/04/23/an-update-to-site-identity-in-desktop-firefox/
Sure enough - thanks. OK, no favicon issue to worry about here, then.
Blocks: 780361
Blocks: 780362
Attached image Ghosts of caret — 
Another issue: caret drawing is not correct in comment 138's linked tryserver build.

 - In a text box, the caret is initially drawn as expected, with a width of two pixels, but when it is erased (either to make it blink, or because its position changes using back), most of the times only half of the pixels are erased, leaving behind a one-pixel wide "ghost" of the caret.

 - Sometimes, after moving the caret forward and back in a text, it will draw at half the size (one pixel width instead of two pixels).

I attach a screenshot of the caret (ghosts) one can see after erasing some text with backspace.


PS: I've filed bug 780361 regarding the WebGL issue, and bug 780362 regarding the Canvas issue.
(In reply to FX from comment #144)
> Created attachment 648972 [details]
> Ghosts of caret
> 
> Another issue: caret drawing is not correct in comment 138's linked
> tryserver build.

Interesting... do you have h/w acceleration disabled? I was unable to reproduce this with acceleration enabled, but after disabling it I see the issue here.
> Interesting... do you have h/w acceleration disabled? I was unable to
> reproduce this with acceleration enabled, but after disabling it I see the
> issue here.

I have "Use hardware acceleration when available" checked. I don't suppose there's any reason it's not supported on my MacBook Pro Retina…

It happens more often if you backspace entire words (keys: alt + backspace) than single letters.
Strange - I still can't reproduce with acceleration enabled (also on a Retina MBP). I wonder what's different.

There's another issue I've observed that may be related, though: when I drag selected text, many of the glyphs in the drag image look as though they've had the rightmost column of pixels clipped off. Looks like it could be a rounding error (truncating instead of rounding the glyph widths when drawing, or something like that).
You probably need to do ScaleToOutsidePixels instead of ScaleToNearestPixels somewhere.
Been using the build from comment 138 for a couple days...

For the most part it seems pretty reliable on the MacBook Pro Retina display... when there's a second or third monitor (low-resolution), I notice some slight glitches when switching a window between monitors:

There seems to be a brief flash of improperly-scaled content, immediately replaced by new, correctly-scaled content. Every once in a while the window turns plain white, which is resolved by switching the window to the other screen again.


Haven't seen any caret issues. (with acceleration enabled)
On the Retina display there are some issues with plugins, but both Flash videos and Google+ Hangout appear to basically work. Flash videos seem to get high-resolution display without understanding it (tiny video controls), and right-click to open the flash menu opens a popup menu at the wrong screen position (some coordinate needs to be scaled?)
I have also been using the build from comment 138 since it was built on my MBP Retina with major issues.  As Brion has mentioned, Flash is working but seems to be displaying hi-res/full screen images in the usual lo-res/small screen windows and I also have the issue with the right click context menu being a ways off to the bottom right.  The issue with the flash resolution car really be seen in the Google Finance individual stock graphs.

There is a similar issue when trying to move/drag a tab in the tab bar.  The thumbnail image is offset a ways to the bottom right from its normal position.

Strangely, the canvas graphs on AWFY are visibly lower resolution on this build then the regular nightly builds.  The graphs look very sharp on a regular nightly build.
I've pushed another tryserver build that should fix the caret-drawing glitches described in comment 144, AFAICT - at least, I can no longer reproduce the problem, regardless of the hardware acceleration setting. This should be available in an hour or so at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-ddc58138cbc4.

FX, please test this when it's ready and let me know whether it resolves the caret issues for you.
Attached file mov of issue — 
I just noticed that there are some "ghosts" left on many drop-down menus where there are many selections.  This is seen using the build from this morning.
I'm having the same issue as on the first build with the Downloads Manager (it's slowly shrinking).

The drop-down menus bug seems to be fixed though.
I spoke too soon, I'm still having drop-down menus ghosts sometimes, not every time.
Blocks: 764083
Scrolling on this build seems to run at half speed also, compared to the normal firefox build. This makes browsing long pages a little bit tedious.
Updated patch, fixing a few more issues including scroll speed and highlight "ghosts" in drop-downs. Tryserver build should be available shortly at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-895e23d81758.
Attachment #648781 - Attachment is obsolete: true
The nightly build does not render the www.apple.com website properly using retina graphics the way that Safari and Chrome both do. The menu-bar, page text, and the images are still pixelated.
(In reply to Antonio from comment #157)
> The nightly build does not render the www.apple.com website properly using
> retina graphics the way that Safari and Chrome both do. The menu-bar, page
> text, and the images are still pixelated.

That is a different bug; apple's JS is looking for -webkit-device-pixel-ratio to download the 2x images... which Firefox obviously doesn't support.
(In reply to Jonathan Kew (:jfkthame) from comment #156)
> Created attachment 650498 [details] [diff] [review]
> [updated WIP] patch to introduce HiDPI support using integer "screen points"
> 
> Updated patch, fixing a few more issues including scroll speed and highlight
> "ghosts" in drop-downs. Tryserver build should be available shortly at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-
> 895e23d81758.

Looking good here, scrolling is back to normal. YouTube plugins are also easier to start playing, but still switch to HiDPI mode when clicked.
Jonathan, this is looking really good. I've been using it for a while and the only thing I've seen which looks odd is some tooltip/titletip boxes (on link hover) being kind of messed up, like the box has a notched in right border. I don't know how to capture it and though I don't see it in our release builds it could be unrelated to your changes. (mouse over date and name links in the attachments table on this bug page, for an example.) It seems inconsistent and I haven't figured why.
Attached image broken titletip box — 
OK. I managed to get a screenshot.
I cannot reproduce in a regular latest nightly Mac build. Also, it's showing up for tooltips in Firefox chrome as well as content. Still can't figure out why it's not always reproducible on the same elements.
Also seeing SVG failing to render at, for example, http://svg-wow.org/gandhi-quotes/gandhi-quotes.xhtml which seems to work fine in latest official nightly.
(In reply to Asa Dotzler [:asa] from comment #161)
> Created attachment 650798 [details]
> broken titletip box
> 
> OK. I managed to get a screenshot.

That's weird! And now that you've mentioned it, I was able to reproduce almost immediately, although I hadn't noticed it at all until now. I also saw a case where the bottom edge of the titletip was uneven.

(In reply to Asa Dotzler [:asa] from comment #163)
> Also seeing SVG failing to render at, for example,
> http://svg-wow.org/gandhi-quotes/gandhi-quotes.xhtml which seems to work
> fine in latest official nightly.

It looked somewhat flaky to me (I see some odd clipping, misplaced final periods, poorly-spaced final "Gandhi") in standard Nightly as well. But I think the main problem here may be that it's using an embedded SVG font, which we don't support, and therefore the text ends up completely differently-sized than was intended, and doesn't fit into the intended areas. (Compare how it looks in Chrome, for example.)

So, while there may well still be SVG-related issues here, I don't think that particular example is a good basis for testing.
Agreed. Ignore the SVG issue for now. It wasn't concrete. The tooltip issue is also not terribly important if the symptom is limited to that case. I only raise it here (rather than a follow-up bug) in case there's something larger at play that might effect more significant parts of the user experience.
The YouTube player on youtube.com is actually working great (not switching into HiDPI mode)!
Comment on attachment 650936 [details]
Bad redraw when dragged from non-HiDPI to HiDPI display

I've noticed a couple of things when dragging between HiDPI and non-HiDPI:
* Dragging between the two displays makes the window 'jerk' a little bit. I'm assuming it's because it's switching over, but it could be done more gracefully.
* Dragging from non-HiDPI to HiDPI display leaves a ghost (see screenshot). Scrolling down scrubs the page clean and it's then displayed correctly.
Another plugin-related issue: Google+ Hangout (using Google's audio/video plugin) mostly works, but the individual people mini-screens don't show.
The vertical scroll bar target image is much smaller on "long" pages then the current nightly using the latest test build (image is from Planet Mozilla page).  On the current nightly it is much larger.
(In reply to Jonathan Guerin from comment #168)
> Comment on attachment 650936 [details]
> Bad redraw when dragged from non-HiDPI to HiDPI display
> 
> I've noticed a couple of things when dragging between HiDPI and non-HiDPI:
> * Dragging between the two displays makes the window 'jerk' a little bit.
> I'm assuming it's because it's switching over, but it could be done more
> gracefully.

Yes, the transition is a bit ugly.

> * Dragging from non-HiDPI to HiDPI display leaves a ghost (see screenshot).
> Scrolling down scrubs the page clean and it's then displayed correctly.

I suspect this might be a more general problem related to redrawing content when the window is zoomed; I can reproduce something similar when zooming in standard Aurora and Nightly builds, without the HiDPI patches. Filed as bug 782262.
Blocks: 781567
I've been using the last tryserver build on my MBP retina for about a week now.  Flash doesn't work, but the text looks GREAT.  I do have one notable issue that seems like a symptom of scaling not being exactly correct, nested bookmark toolbar folders.  I'll try and upload an example, but when I mouse through the items one level deep of bookmark toolbar items it works ok (but not perfect) but when I get to two levels deep it's really bad and often just clicks off the menu so I can't even mouse to the lower items.

    
    

    
  
Updated tryserver build is at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-d7b36b67a324. I think this will fix the "ghost" described in comment 168; also fixes an issue with popup menus not scaling correctly when you zoom.

    
    

    
  

      
      
      
      

        Attached image
          
        Issue with the Find bar
        — 
      

    


  
Using comment 174's build, I don't see caret ghosts anymore. I have however found another issue in the handling of the "Find" bar at the bottom of the window when cmd+F is pressed: most of the times, then "Find bar" does not display, but only the text field itself (white background, and then black text when other keys are pressed). The attached snapshot shows this.

Then, when the text to find cannot be matched in the page anymore, the "Find bar" is redrawn, and now clearly visible. However,  it's still not perfectly displayed: there are white areas of a few pixels to its left and right (see next attachment).

    
    

    
  
I've not been able to reproduce this Find-bar issue - it seems to appear reliably and draw correctly for me. Do you have any addons installed that might possibly be affecting it?

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #177)
> I've not been able to reproduce this Find-bar issue - it seems to appear
> reliably and draw correctly for me. Do you have any addons installed that
> might possibly be affecting it?

It also happens without add-ons, if the add-on bar is displayed. It does not happen when the add-on bar is not displayed. It does not happen on the first search inside a page, but systematically happens on all subsequent searches.

    
    

    
  
(In reply to FX from comment #178)
> (In reply to Jonathan Kew (:jfkthame) from comment #177)
> > I've not been able to reproduce this Find-bar issue - it seems to appear
> > reliably and draw correctly for me. Do you have any addons installed that
> > might possibly be affecting it?
> 
> It also happens without add-ons, if the add-on bar is displayed. It does not
> happen when the add-on bar is not displayed. It does not happen on the first
> search inside a page, but systematically happens on all subsequent searches.

I can't seem to repro this on the new tryserver build either... I've tried enabling and disabling the add-on bar, but no luck.

    
    

    
  
I can reproduce this with a clean new profile, 100% of the time (I restarted Nightly six times to check), like this:

 1. Display both Bookmarks toolbar and add-ons toolbar
 2. Load https://www.google.com/ in a new window
 3. Make the window less tall so that a vertical scrollbar appears
 4. Search for "face" (which appears few times in the page): cmd+F, then "face"
 5. Hit escape to dismiss the search bar
 6. Search for "face" again ==> NOW THAT DOESN'T WORK PROPERLY

    
    

    
  
(In reply to FX from comment #180)
> I can reproduce this with a clean new profile, 100% of the time (I restarted
> Nightly six times to check), like this:
> 
>  1. Display both Bookmarks toolbar and add-ons toolbar
>  2. Load https://www.google.com/ in a new window
>  3. Make the window less tall so that a vertical scrollbar appears
>  4. Search for "face" (which appears few times in the page): cmd+F, then
> "face"
>  5. Hit escape to dismiss the search bar
>  6. Search for "face" again ==> NOW THAT DOESN'T WORK PROPERLY

I am unable to reproduce the bug with the latest try build, new window, toolbars open, short window: http://dl.dropbox.com/u/1736241/Screenshots/p1.png

    
    

    
  
Aha! I've now been able to reproduce the Find-bar problem as described, but *only* when I explicitly disable hardware acceleration (and restart the browser, so that change takes effect). With acceleration enabled, I can't get it to happen.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #182)
> Aha! I've now been able to reproduce the Find-bar problem as described, but
> *only* when I explicitly disable hardware acceleration

Indeed, that's the second time this happens in this PR, so I double checked. So, I have the "Use hardware acceleration when available" setting checked, but in about:support Firefox Nightly says:

Graphics
Vendor ID: 0x10de
Device ID: 0x fd5
WebGL Renderer: NVIDIA Corporation -- NVIDIA GeForce GT 650M OpenGL Engine -- 2.1 NVIDIA-8.0.51
GPU Accelerated Windows: 0
Azure Canvas Backend: quartz
Azure Fallback Canvas Backend: none
Azure Content Backend: none

(in fact, Firefox 14.0.1 says the same thing)  I find it very surprising because this is good hardware (brand new MacBook Pro Retina). I'll try and file a separate bug about this.

    
    

    
  
Interesting - I wonder why that's happening. Yes, please do file a separate bug for it.

FWIW, I'm also using a retina-MBPro, but my about:support reports a different NVIDIA driver version (and OpenGL is used as expected):

Graphics
Vendor ID: 0x10de
Device ID: 0x fd5
WebGL Renderer: NVIDIA Corporation -- NVIDIA GeForce GT 650M OpenGL Engine -- 2.1 NVIDIA-7.24.10
GPU Accelerated Windows: 1/1 OpenGL
AzureCanvasBackend: quartz
AzureFallbackCanvasBackend: none
AzureContentBackend: none

    
    

    
  
This might be a dumb question, but are either of you perhaps running Mountain Lion?

    
    

    
  
I am unable to reproduce the issue. My hardware acceleration seems to be enabled and working fine and I have the same driver version as the person with no hwa.

I am running Mountain Lion. 

Graphics
Vendor ID: 0x10de
Device ID: 0x fd5
WebGL Renderer: NVIDIA Corporation -- NVIDIA GeForce GT 650M OpenGL Engine -- 2.1 NVIDIA-8.0.51
GPU Accelerated Windows: 1/1 OpenGL
AzureCanvasBackend: quartz
AzureFallbackCanvasBackend: none
AzureContentBackend: none

I suspect this is an issue for a different bug report.

    
    

    
  
I suggest trying a new profile in case you have some non default preferences.

    
    

    
  
(In reply to Asa Dotzler [:asa] from comment #186)
> I am unable to reproduce the issue. My hardware acceleration seems to be
> enabled and working fine and I have the same driver version as the person
> with no hwa.

The difference appears to be that I open Firefox in 32-bit mode, in order for PDF to display natively in the browser (the Firefox PDF Plugin for Mac OS, which is 32-bit only). I have filed bug 782713 to track this.

    
    

    
  
Here's my Graphics (ML installed):

Graphics
Vendor ID0x10de
Device ID0x fd5
WebGL Renderer NVIDIA Corporation -- NVIDIA GeForce GT 650M 
OpenGL Engine -- 2.1 NVIDIA-8.0.51
GPU Accelerated Windows1/1 OpenGL
AzureCanvasBackend quartz
AzureFallbackCanvasBackend none
AzureContentBackend none

    
    

    
  
The new tryserver build appears to have serious trouble with Magic Mouse gestures (back and forwards)(trackpad ones seem OK). If I use the same gestures on Safari, they work fine. Not sure what's going on.

    
    

    
  
(In reply to Jonathan Guerin from comment #190)
> The new tryserver build appears to have serious trouble with Magic Mouse
> gestures (back and forwards)(trackpad ones seem OK). If I use the same
> gestures on Safari, they work fine. Not sure what's going on.

Just checked Firefox 14 and gestures work fine there too.

    
    

    
  
(In reply to comment #190)

This is probably bug 782552 (and therefore unrelated to Jonathan's work here).

You should see the same problem in today's mozilla-central nightly.

    
    

    
  
Thanks for the update!

    
    

    
  
Previous to 17.0a1 (2012-08-14), setting layers.acceleration.disabled to true would cause the text to look beautifully crisp and sharp on Retina displays.

The 17.0a1 build broke this behaviour. Text is now blurry with acceleration enabled or disabled.

    
    

    
  
Confirmed - the last two Nightlies have broken HiDPI support.
@Carsten, If you grab the .dmg from two nights ago it's still nice and crisp:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-08-13-03-05-32-mozilla-central/

Hope it's fixed soon though, as I'd rather have the benefit of all the other checkins... (although I realise Nightlies are potentially unstable, that's the point!)

    
    

    
  
(In reply to comment #194 and comment #195)

I'll bet this is because one or more parts of the patch for bug 539356 (which were landed and backed out a few weeks ago) were relanded on 2012-08-13.

See comment #95 and comment #96 above.
Depends on: 564815

    
    

    
  


  
I'm posting an updated set of WIP patches. These are getting close to working, I think, with the notable exception of plugin issues.

Part 1 is the basic support for "screen points" as the coordinate system for widget management, potentially distinct from device pixels, although by itself this doesn't enable actual HiDPI rendering.

    
    

    
  


  
The a11y changes from the screen-points patch, split out for my convenience. This really hasn't been tested yet.

    
    

    
  


  
This actually turns on HiDPI mode on OS X (when appropriate).

    
    

    
  


  
This is essentially Markus's patch for native theme rendering, with pretty minimal updates/rebasing.

    
    

    
  


  
And this is also Markus's patch, including in the series here for completeness.

    
    

    
  


  
This provides a setting "layout.hidpi.enabled" in about:config that can be used to turn HiDPI support on/off for testing purposes. (Requires a browser restart for the change to take effect.)

    
  

        Attachment #650498 -
        Attachment is obsolete: true

    
    

    
  
Some of these don't apply cleanly to current trunk :-(

    
    

    
  
Sigh.... sorry about that. Yeah, they tend to bitrot pretty quickly. My tree is currently based on m-c rev f89feda9d997, which is dated Aug 12th. I'll try to rebase over the weekend and post updates as needed.

    
    

    
  
I've got updates for parts 1, 3 and 5.  I should shortly have one for part 6 (which needs to land on top of part 3).

I'll post these once I'm sure they work.

(Parts 2 and 4 should apply, though maybe at offsets.)

    
    

    
  


  
This should be applied on top of parts 1, 3 and 5 together.

        Attachment #652866 -
        Attachment is obsolete: true

    
    

    
  
> This should be applied on top of parts 1, 3 and 5 together.

Or parts 1, 2, 3, 4 and 5 together.

I've started a tryserver build of all 6 parts.

    
    

    
  
(In reply to Steven Michaud from comment #211)
> Here's the tryserver build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-
> 53e54134e056/try-macosx64/firefox-17.0a1.en-US.mac.dmg
> 
> Amazingly, there were *no* test failures (even spurious ones).

This is working great, everything is still HiDPI, except for plugins.

No issues with trackpad gestures anymore, except that gestures are slightly broken in that you have to click into the tab in order to get them to register properly.

    
    

    
  

      
      
      
      

        Attached image
          
        Low-res Chrome
        — 
      

    


  
It looks like the Chrome isn't yet rendering in HiDPI mode? The tab curves are very pixelated, especially when you compare them to Chrome's look.

    
    

    
  
(In reply to Jonathan Guerin from comment #213)
> Created attachment 653230 [details]
> Low-res Chrome
> 
> It looks like the Chrome isn't yet rendering in HiDPI mode? The tab curves
> are very pixelated, especially when you compare them to Chrome's look.

The first two 'chrome's weren't supposed to be capitalised. Apologies for the confusion.

    
    

    
  
(In reply to Jonathan Guerin from comment #213)
> It looks like the Chrome isn't yet rendering in HiDPI mode? The tab curves
> are very pixelated, especially when you compare them to Chrome's look.

At least some of this is being tracked in bug 781327.

    
    

    
  
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #92)
> I still lean towards avoiding that by making all widget APIs use device
> pixels and adjusting Cocoa coordinates to device pixels in the Mac widget
> code.

To do things like answer "which widget is under (or nearest to) the mouse?", we really need widget coordinates to operate in a single, continuous coordinate system that spans the entire visible desktop space. The screen-points approach provides that; widget APIs that use device (backing-store) pixels don't, and that makes life really hard.

    
    

    
  


  
I don't recall this happening prior to the new patches, but the social buttons present on many sites are not scaling properly. Verified the same site in Chrome and Safari and they have no issues.

Source URI: http://www.wired.com/wiredscience/2012/08/curiositys-secrets/?pid=4630&viewall=true

    
    

    
  
(In reply to comment #217)

This is probably more relevant to bug 781327.

    
    

    
  
Yes, this seems to be a new issue; I don't see the problem in the tryserver build from Aug 13th (comment 174).

    
    

    
  


  
(In reply to Steven Michaud from comment #211)
> Here's the tryserver build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-
> 53e54134e056/try-macosx64/firefox-17.0a1.en-US.mac.dmg

Trying this out, mostly seems ok...

I did find an odd problem when detaching an external (non-HiDPI) screen that my Gmail tab is getting rendered at half-size when on the internal screen. This isn't just a quick flash -- it's persistent, and the tab remains interactive.

It looks like much of the Gmail interface is actually in an iframe -- this might be similar to the social networking button problems mentioned above, if they use embedded iframes (looks like the Google+ one does at least).

    
    

    
  
(In reply to Brion Vibber from comment #220)
> Created attachment 653481 [details]
> Screenshot of half-size Gmail after detaching external monitor
> 
> (In reply to Steven Michaud from comment #211)
> > Here's the tryserver build:
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-
> > 53e54134e056/try-macosx64/firefox-17.0a1.en-US.mac.dmg
> 
> Trying this out, mostly seems ok...
> 
> I did find an odd problem when detaching an external (non-HiDPI) screen that
> my Gmail tab is getting rendered at half-size when on the internal screen.
> This isn't just a quick flash -- it's persistent, and the tab remains
> interactive.
> 
> It looks like much of the Gmail interface is actually in an iframe -- this
> might be similar to the social networking button problems mentioned above,
> if they use embedded iframes (looks like the Google+ one does at least).

Just noticed that Gmail is also doing this for me. Didn't detach or anything, brand new tab.

    
    

    
  


  
I noticed the half-size problem for Gmail but it's not only there.  Can reproduce this reliably by having the browser on my external monitor, detaching it, then zooming in.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #216)
> To do things like answer "which widget is under (or nearest to) the mouse?",
> we really need widget coordinates to operate in a single, continuous
> coordinate system that spans the entire visible desktop space. The
> screen-points approach provides that; widget APIs that use device
> (backing-store) pixels don't, and that makes life really hard.

Where do we do that? If we only do it in Cocoa code, that code can use Cocoa's coordinate system.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #219)
> Yes, this seems to be a new issue; I don't see the problem in the tryserver
> build from Aug 13th (comment 174).

The problem with the "social buttons" on wired.com is a result of changes made in bug 775965 - in particular, cset 0cfcc4e860c5. I suspect this may account for the Gmail problem as well, given that iframes seem to be a factor in all these issues.

    
    

    
  
Any chance of getting a new tryserver build with the patches? :)

    
    

    
  


  
I'm not sure if this is the right place to put this (please redirect me to the correct bug if not), but the current tryserver build has some weird inactive window behaviour. Looking at how other apps change their colours when they become inactive, Firefox's current behaviour is weird. The tabs don't switch to the lighter grey, which makes the window look 'wrong'. Don't know if it's a bug or by design.

Cheers,

Jonathan

    
    

    
  
(In reply to Jonathan Guerin from comment #225)
> Any chance of getting a new tryserver build with the patches? :)

I've started a new build including a workaround for the iframe/social-button scaling problem; it'll be a couple hours before it's ready, though. See https://tbpl.mozilla.org/?tree=Try&rev=d480af0f9f0d for progress.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #228)
> (In reply to Jonathan Guerin from comment #225)
> > Any chance of getting a new tryserver build with the patches? :)
> 
> I've started a new build including a workaround for the iframe/social-button
> scaling problem; it'll be a couple hours before it's ready, though. See
> https://tbpl.mozilla.org/?tree=Try&rev=d480af0f9f0d for progress.

Just tried the OSX64 build, confirm that it does fix scaling issues in both Gmail and Share buttons. Cheers!
Blocks: 784909

    
    

    
  
Confirming this build works great, though is rather lagged on long webpages or complex web apps, and favicons are blurry -- though that is to be expected as they wouldn't be hi-res.

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-d480af0f9f0d/try-macosx64/firefox-17.0a1.en-US.mac.dmg

    
  
Blocks: osx-hidpi

    
  

        Attachment #652860 -
        Attachment is obsolete: true

    
  

        Attachment #652862 -
        Attachment is obsolete: true

    
  

        Attachment #652967 -
        Attachment is obsolete: true

    
  

        Attachment #652970 -
        Attachment is obsolete: true

    
  

        Attachment #652972 -
        Attachment is obsolete: true

    
  

        Attachment #652974 -
        Attachment is obsolete: true

    
    

    
  
Updated patch series to latest trunk code, and (I hope) fixed a few remaining test failures across other platforms. Tryserver build in progress at https://tbpl.mozilla.org/?tree=Try&rev=9c638aa4ae30.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #237)
> Updated patch series to latest trunk code, and (I hope) fixed a few
> remaining test failures across other platforms. Tryserver build in progress
> at https://tbpl.mozilla.org/?tree=Try&rev=9c638aa4ae30.

Using build now, everything looking good! :)

    
    

    
  
Jonathan, this is looking great. Are you going to be able to get reviews and get this landed for the 17 uplift?

    
    

    
  


  
Using the latest try build above...

Things are getting into an inconsistent state if I tear off a tab from a window on a screen at one resolution onto a screen with a different density -- this is a screenshot of Gmail rendered on a non-HiDPI external screen after tearing the tab off from a window on the internal Retina screen.

It appears that graphics and layout are roughly correct, but text is all double-sized in this case.

After moving the new window across a screen boundary, it returns to normal.

    
    

    
  


  
Similar to the case of tearing off a tab from the Retina to the external display, but text is tiny instead of huge. Again, moving the window across a screen density boundary resolves the issue.

This all happens on simpler pages like Bugzilla as well, not just Gmail with its fancy iframes.

    
    

    
  
For me, in the latest tryserver build (as with the others so far), embedded Flash still seems broken - all controls are at, presumably, actual pixel size rather than proportionate, and the controls when right-clicking appear far away from the embedded object.

I'm using Mac OS 10.8, on a RetinaBook with display scaled to 1920x1200, which may be significant.

Steps to reproduce: View any YouTube video (Flash not HTML5!). Right-click in the video area to see the right-click menu appear to the right and down from the mouse cursor.
Also see the Vimeo and Bandcamp embeds here for instance:
http://earslend.blogspot.com

    
    

    
  
(In reply to Peter Hollo from comment #242)
> For me, in the latest tryserver build (as with the others so far), embedded
> Flash still seems broken - all controls are at, presumably, actual pixel
> size rather than proportionate, and the controls when right-clicking appear
> far away from the embedded object.
> 
> I'm using Mac OS 10.8, on a RetinaBook with display scaled to 1920x1200,
> which may be significant.
> 
> Steps to reproduce: View any YouTube video (Flash not HTML5!). Right-click
> in the video area to see the right-click menu appear to the right and down
> from the mouse cursor.
> Also see the Vimeo and Bandcamp embeds here for instance:
> http://earslend.blogspot.com

This is tracked separately in 781567

    
    

    
  
(In reply to comment #243)

Actually problems with plugins in HiDPI mode are being worked on at bug 785667.

    
    

    
  
(In reply to Steven Michaud from comment #244)
> (In reply to comment #243)
> 
> Actually problems with plugins in HiDPI mode are being worked on at bug
> 785667.

Oops - should the other be closed as a dupe?

        Attachment #635247 -
        Flags: feedback?(roc)
Depends on: 788907

    
    

    
  


  
This is an alternative patch for the HiDPI issue, avoiding the introduction of "screen points" in layout and instead pushing as much of the work as possible down into the Cocoa widget code.

A tryserver build with this patch is in progress at https://tbpl.mozilla.org/?tree=Try&rev=50af84dce998.

Some known problems with this build include:

- the drag image is misplaced when "tearing-off" a tab, or when dragging a very large selection (such that the drag image gets scaled down).

- the downloads panel seems to have positioning/sizing problems, although I think there may be some current problems with this panel in Nightly, so perhaps that's not purely the fault of the HiDPI patch.

- dynamic resolution switching and multi-screen support is broken; e.g. trying to move windows between a Retina screen and a separate non-Retina display will result in thoroughly garbled display. Just don't do that.

- alerts shown at startup (e.g. the profile manager, or the "copy of Firefox is already open" message) appear at half size.

- plugin interactions will still be broken (requires bug 785667).

It should work pretty well for non-plugin browsing on a Retina MBPro, though. You can turn off HiDPI mode with the about:config setting "gfx.hidpi.enabled" (requires a restart) to compare low- and hi-res modes.

    
    

    
  


  
The latest tryserver (50af84dce998) is running in full resolution mode, rather than HiDPI mode, making it effectively unusable. Maybe I misunderstood the intent of this build, but I certainly can't run it in order to report any issues to you due to the tiny, tiny content :)

    
    

    
  
Argh - sorry, I omitted to change a default preference that you need to adjust. Go to about:config and set layout.css.devPixelsPerPx to -1 and I think you'll find it more usable!

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #248)
> Argh - sorry, I omitted to change a default preference that you need to
> adjust. Go to about:config and set layout.css.devPixelsPerPx to -1 and I
> think you'll find it more usable!

Much better! I'll report back if I see anything broken other than your known issues. Anything that I should be focusing on?

    
    

    
  
Aside from those issues, I think it should be pretty usable, so please report anything that seems wrong (especially issues related to resolution/scaling/etc - if something looks like it might be a more general problem, see if it occurs in a current Nightly build without the HiDPI patch, and/or with gfx.hidpi.enabled set to false).

One other thing I've seen locally is that occasionally the Downloads and Home buttons in the toolbar start displaying at the wrong size (the buttons expand to the full height of the toolbar and get a two-pixel thick outline, although their icons remain correct). Reloading the page and slightly resizing the window seems to fix this. I haven't pinned down what triggers this problem, though, so if you see it and can reliably reproduce it, I'd like to know the precise steps.

    
    

    
  
I won't be able to test this until resolution switching is fixed as I usually use multiple monitors when working; will test the next version.

    
    

    
  
Comment on attachment 658898 [details] [diff] [review]
patch, implement HiDPI rendering in Cocoa widget code

Review of attachment 658898 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsDOMWindowUtils.cpp
@@ +1438,5 @@
>  
>  NS_IMETHODIMP
>  nsDOMWindowUtils::GetScreenPixelsPerCSSPixel(float* aScreenPixels)
>  {
> +  *aScreenPixels = 1.0;

1.0f

::: layout/style/nsRuleNode.cpp
@@ +2991,5 @@
> +#ifdef XP_MACOSX
> +      // Interpret the Cocoa "point size" as CSS pixels
> +      // (NOT equal to device pixels, when running in HiDPI)
> +      systemFont.size =
> +        nsPresContext::CSSPixelsToAppUnits((float)fontStyle.size);

Probably should just pass dev-pixels-per-CSS-pixel as a parameter to LookAndFeel::GetFont and convert to dev pixels in the Mac LookAndFeel code.

::: widget/nsIWidget.h
@@ +1155,5 @@
> +     * Return the client size of the widget in device pixels. This is the
> +     * size that will be used for viewport construction on accelerated
> +     * backends.
> +     */
> +    virtual nsIntSize ClientSurfaceSize() = 0;

How is this different from GetClientBounds? I thought GetBounds/GetScreenBounds/GetClientBounds would all return device pixels?

    
    

    
  
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #252)
> Comment on attachment 658898 [details] [diff] [review]
> patch, implement HiDPI rendering in Cocoa widget code
> 
> Review of attachment 658898 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/base/nsDOMWindowUtils.cpp
> @@ +1438,5 @@
> >  
> >  NS_IMETHODIMP
> >  nsDOMWindowUtils::GetScreenPixelsPerCSSPixel(float* aScreenPixels)
> >  {
> > +  *aScreenPixels = 1.0;
...

Note that the change to nsDOMWindowUtils::GetScreenPixelsPerCSSPixel here actually changes its semantics, which may or may not be acceptable. It breaks gfx/tests/test_bug513439.html (testcase for changes to the devPixelsPerPx setting).

Currently, the value returned by GetScreenPixelsPerCSSPixel seems to reflect both the devPixelsPerPx value *and* the page zoom ratio. However, browser.js and other code treats it as page zoom ratio alone; only test_bug513439.html seems to expect it to reflect devPixelsPerPx (and I guess it would probably fail if run with a non-1.0 zoom ratio).

One possibility to resolve this may be introducing a new GetZoomRatio API which unambiguously returns the page zoom ratio, and changing existing clients of GetScreenPixelsPerCSSPixel to use this when it's really zoom they want to know. And then test_bug513439.html will be able to divide the (original) GetScreenPixelsPerCSSPixel value by the zoom ratio in order to correctly check the devPixelsPerPx setting.

    
    

    
  
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #252)
> ::: widget/nsIWidget.h
> @@ +1155,5 @@
> > +     * Return the client size of the widget in device pixels. This is the
> > +     * size that will be used for viewport construction on accelerated
> > +     * backends.
> > +     */
> > +    virtual nsIntSize ClientSurfaceSize() = 0;
> 
> How is this different from GetClientBounds? I thought
> GetBounds/GetScreenBounds/GetClientBounds would all return device pixels?

Yes - this is a legacy of how the patch has evolved, but I think we can eliminate it again.

    
    

    
  
Comment on attachment 654977 [details] [diff] [review]
pt 1 - basic support for 'screen points' as widget coordinate system

FYI, bug 564815 just landed in inbound which moves around the code from nsDOMWindowUtils::GetScreenPixelsPerCSSPixel, so this patch will need some simple rebasing.

    
    

    
  
I want to get HIDPI version firefox!

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #253)
> One possibility to resolve this may be introducing a new GetZoomRatio API
> which unambiguously returns the page zoom ratio, and changing existing
> clients of GetScreenPixelsPerCSSPixel to use this when it's really zoom they
> want to know. And then test_bug513439.html will be able to divide the
> (original) GetScreenPixelsPerCSSPixel value by the zoom ratio in order to
> correctly check the devPixelsPerPx setting.

That sounds right.

    
    

    
  


  
Updated version of the Cocoa-widget HiDPI patch, including zoomFactor fix (now looks like it'll pass all tests on try, although that doesn't include actually running in HiDPI mode!) https://tbpl.mozilla.org/?tree=Try&rev=6b773f738a34

Multi-screen/mixed-resolution support is not in place yet.

        Attachment #658898 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 660067 [details] [diff] [review]
patch v2, implement HiDPI rendering in Cocoa widget code

Roc, any more feedback at this stage would be welcome. As I understand it, you'd strongly prefer this over the screen-points patches, right? (Note that we're still going to need to hack nsXULWindow and friends substantially to deal with the multi-screen issues.)

        Attachment #660067 -
        Flags: feedback?(roc)

    
    

    
  
I'm waiting until 785667 picks up this patch, as it's working great for me (I don't need to switch between monitors at the moment).

    
    

    
  
How can I get a build of the latest patched version so I can test it?

    
    

    
  
You would need to clone hg.mozilla.org/mozilla-central apply the patch and compile Firefox. https://developer.mozilla.org/en-US/docs/Simple_Firefox_build

    
    

    
  
(In reply to Arthur Roussel from comment #261)
> How can I get a build of the latest patched version so I can test it?

You can get the tryserver build mentioned in comment 258 from https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-6b773f738a34/try-macosx64/firefox-18.0a1.en-US.mac.dmg. This does _not_ include plugin support from bug 758667, though.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #263)
> (In reply to Arthur Roussel from comment #261)
> > How can I get a build of the latest patched version so I can test it?
> 
> You can get the tryserver build mentioned in comment 258 from
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-
> 6b773f738a34/try-macosx64/firefox-18.0a1.en-US.mac.dmg. This does _not_
> include plugin support from bug 758667, though.

You mean bug 785667, related to plugins, I suppose.

    
    

    
  
I found the following bug on this build. 

Not sure if this is already covered via plugins, or is caused by this work. But have not been able to find it reported elsewhere in bugzilla.

Steps: 
1. customize the toolbar. (View -> Toolbars -> Customize...)
2. The toolbar customization appears to open onscreen, then disappears.
3. The user is unable to affect the menu and url bar.
4. Remnants of the customization screen flash intermittently when the user attempts to interact with the window.

Note:
I was able to get the customize screen to show up normally, by right clicking on the mouse, however this is highly unintuitive.

    
    

    
  
I have the exact same problem reported by josh.ellinger

    
    

    
  
(In reply to comment #265 and comment #266)

See bug 785667 comment #9 and bug 785667 comment #10 for something that's possibly related.

Before reporting any problems here, please check that you don't also seem them in recent trunk nightlies.  If the problem happens in a current nightly, it (of course) has nothing to do with these patches.

Please, no more "me too" comments, unless you have corrections or additional information.

    
    

    
  
(In reply to Steven Michaud from comment #267)
I can confirm that the problem does not occur on the current nightly. It is possible as you mention it is related to the bug in comment #9 and #10.

    
    

    
  
Comment on attachment 660067 [details] [diff] [review]
patch v2, implement HiDPI rendering in Cocoa widget code

Review of attachment 660067 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, I do like this a lot better :-).

::: dom/interfaces/base/nsIDOMWindowUtils.idl
@@ +688,5 @@
>  
>    /**
> +   * Get the current zoom factor.
> +   */
> +  readonly attribute float zoomRatio;

The changes that introduce this value, and use it in browser.js (and elsewhere?), should be split out into their own patch (applied before the main patch).

::: gfx/layers/ipc/CompositorParent.cpp
@@ +915,5 @@
>  
>    // mWidget doesn't belong to the compositor thread, so it should be set to
>    // NULL before returning from this method, to avoid accessing it elsewhere.
>    nsIntRect rect;
> +  mWidget->GetClientBounds(rect);

This change isn't really needed for this bug, right? I don't mind landing it as part of this bug, but it can go in its own patch (along with the similar changes in LayerManagerOGL (and elsewhere?)).

::: widget/LookAndFeel.h
@@ +496,3 @@
>     */
> +  static bool GetFont(FontID aID, nsString& aName, gfxFontStyle& aStyle,
> +                      nsPresContext *aPresContext);

The change to add this parameter and pass it where needed can be split out into its own patch.

        Attachment #660067 -
        Flags: feedback?(roc) → feedback+

    
    

    
  
Jonathan: any updates here?

While work on bug 785667 is ongoing, would it be possible to land this disabled by a pref? Would be good to get some wider opt-in testing with Nightly, even if plugins won't be right yet.

    
    

    
  
+1

I've stopped dogfooding Nightly and reluctantly use Beta 16 because that is the last channel that allows the layers.acceleration.disabled=true pref to display high-resolution fonts (as per comment 195). I'm willing to eat more dogfood than most users, so if *I* can't deal with blurry text, I worry we might be bleeding many Retina users to other browsers.

    
    

    
  
(In reply to Justin Dolske [:Dolske] from comment #270)
> Jonathan: any updates here?
> 
> While work on bug 785667 is ongoing, would it be possible to land this
> disabled by a pref? Would be good to get some wider opt-in testing with
> Nightly, even if plugins won't be right yet.

+1 Yes Please. The blurry text is driving me crazy, and like the commenter before, it's beginning to drive to stray to other browsers for the time being.

    
    

    
  
+1 (currently using a HiDPI Nightly build I got from try servers about a week ago)

R G Porter: you can use this build for now https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-6b773f738a34/try-macosx64/firefox-18.0a1.en-US.mac.dmg There are some quirks (with Flash mostly) but overall it works pretty well.

    
    

    
  
Also try the builds at bug 785667, which incorporate this bug's patches (the screenpoints and "cocoapoints" patches).

    
    

    
  


  

        Attachment #663479 -
        Flags: review?(roc)

    
  

        Attachment #660067 -
        Attachment is obsolete: true

    
  

        Attachment #663479 -
        Flags: review?(smichaud)

    
  

        Attachment #654977 -
        Attachment is obsolete: true

    
  

        Attachment #654978 -
        Attachment is obsolete: true

    
  

        Attachment #654979 -
        Attachment is obsolete: true

    
  

        Attachment #654980 -
        Attachment is obsolete: true

    
  

        Attachment #654981 -
        Attachment is obsolete: true

    
    

    
  
The patch series 1-6 above seems to be working pretty well on a retina-display MacBook. Multiple screens with differing backing-resolutions don't work well yet, so the patch is currently configured to disable hidpi on such setups; I'm working on a further patch to address this, but would like to get the basic support here landed ASAP.

(Parts 2 and 3 here are just minor clean-up; not actually essential to this bug, but things I happened to stumble across during the evolution of the main patch. Part 1 is Markus's original theme-code patch, just rebased as needed; those changes are independent of the main hidpi-content support.)

Tryserver build with this patch queue (note: this does NOT include plugin support from bug 785667) is in progress at https://tbpl.mozilla.org/?tree=Try&rev=c3817811fb07.

    
    

    
  
When is it expected that the two patches will merge...?

    
    

    
  
> When is it expected that the two patches will merge...?

Probably not til they land.  But I'm about to post a new patch (and tryserver build) at bug 785667.

    
    

    
  
Comment on attachment 663481 [details] [diff] [review]
pt 1 - Mac OS X native theme rendering support for HiDPI display.

This looks fine to me.  It's also done fine in my testing.

        Attachment #663481 -
        Flags: review?(smichaud) → review+

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #281)
> Tryserver build with this patch queue (note: this does NOT include plugin
> support from bug 785667) is in progress at
> https://tbpl.mozilla.org/?tree=Try&rev=c3817811fb07.

I am seeing a few pretty consistent bugs:

1) The Customization panel opens off-screen with no way to close it or use it

2) The awesomebar results panel is frequently the full height of the screen even if the results don't fill it up; also something I can't reproduce is that sometimes it ends up off screen in the same way as the customization panel

3) Expanding the folder/tags selections in the bookmark panel makes it huge and it doesn't shrink again even if you collapse the extended selection UI

4) The downloads panel is very wide

    
    

    
  
(In reply to Stephen Horlander from comment #285)
> 1) The Customization panel opens off-screen with no way to close it or use it

Ugh! Yes, I see the same thing.

> 2) The awesomebar results panel is frequently the full height of the screen
> even if the results don't fill it up; also something I can't reproduce is
> that sometimes it ends up off screen in the same way as the customization
> panel

I haven't been able to reproduce this; it seems to be consistently fine for me. Are you on a multi-screen configuration?

> 3) Expanding the folder/tags selections in the bookmark panel makes it huge
> and it doesn't shrink again even if you collapse the extended selection UI

Haven't been able to reproduce this either.

> 4) The downloads panel is very wide

To clarify, are you referring to the "doorhanger" panel that appears from the toolbar button, or the separate window (Tools/Downloads)? I've been seeing some erratic sizing issues with the former.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #286)
> (In reply to Stephen Horlander from comment #285)
> > 1) The Customization panel opens off-screen with no way to close it or use it
> 
> Ugh! Yes, I see the same thing.

Possible workaround: set toolbar.customization.usesheet to false. The non-sheet version seems to stay where it belongs.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #286)
> > 2) The awesomebar results panel is frequently the full height of the screen
> > even if the results don't fill it up; also something I can't reproduce is
> > that sometimes it ends up off screen in the same way as the customization
> > panel
> 
> I haven't been able to reproduce this; it seems to be consistently fine for
> me. Are you on a multi-screen configuration?

It isn't doing it right now but if it happens again I will get a screen shot and try to figure out what caused it.

I am not on a multi-screen configuration.


> > 3) Expanding the folder/tags selections in the bookmark panel makes it huge
> > and it doesn't shrink again even if you collapse the extended selection UI
> 
> Haven't been able to reproduce this either.

Here is what I am seeing: http://cl.ly/image/1d0X0c1Q1z2R


> > 4) The downloads panel is very wide
> 
> To clarify, are you referring to the "doorhanger" panel that appears from
> the toolbar button, or the separate window (Tools/Downloads)? I've been
> seeing some erratic sizing issues with the former.

Yes, the panel in nightly that is anchored to the toolbar button.

    
    

    
  
Comment on attachment 663476 [details] [diff] [review]
pt 4 - pass the current nsPresContext to LookAndFeel code when requesting a font.

Review of attachment 663476 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I realize now that it's a bit of a layering violation to pass nsPresContext into widget. Passing the device-pixels-per-CSS-pixel ratio through instead is the better way to go. Sorry!

    
  

        Attachment #663476 -
        Attachment is obsolete: true

        Attachment #663476 -
        Flags: review?(roc)

    
    

    
  
Comment on attachment 663479 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Cancelling r? on this part until I resolve the problem with positioning sheet windows (Configure, Edit Bookmark) relative to the browser window.

        Attachment #663479 -
        Flags: review?(smichaud)

        Attachment #663479 -
        Flags: review?(roc)

    
    

    
  
Comment on attachment 663478 [details] [diff] [review]
pt 5 - provide a zoomRatio API in nsIDOMWindowUtils, and use this rather than inferring zoom from CSS to device pixel ratio.

Review of attachment 663478 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/interfaces/base/nsIDOMWindowUtils.idl
@@ +689,5 @@
>  
>    /**
> +   * Get the current zoom factor.
> +   */
> +  readonly attribute float zoomRatio;

Actually I think this is just the same as nsIMarkupDocumentViewer.fullzoom. So I think we don't need to add this here.

    
    

    
  
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #292)

> Actually I think this is just the same as nsIMarkupDocumentViewer.fullzoom.
> So I think we don't need to add this here.

It's not quite the same, because fullzoom doesn't reflect the quantization that happens when we implement the zoom ratio by adjusting the appUnits per device pixel.

    
    

    
  
Comment on attachment 663478 [details] [diff] [review]
pt 5 - provide a zoomRatio API in nsIDOMWindowUtils, and use this rather than inferring zoom from CSS to device pixel ratio.

Review of attachment 663478 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/interfaces/base/nsIDOMWindowUtils.idl
@@ +689,5 @@
>  
>    /**
> +   * Get the current zoom factor.
> +   */
> +  readonly attribute float zoomRatio;

OK. Rename this to fullZoom then and add a comment that it's approximately the same as nsIMarkupDocumentViewer.fullZoom but takes into account quantization (and/or other limits imposed by Gecko).

        Attachment #663478 -
        Flags: review?(roc) → review+

    
    

    
  


  

        Attachment #664447 -
        Flags: review?(smichaud)

    
  

        Attachment #663479 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 635246 [details] [diff] [review]
native theme rendering changes

Just obsoleting an old copy of this patch, though we're carrying it forward as "part 1" of the full series.

        Attachment #635246 -
        Attachment is obsolete: true

        Attachment #635246 -
        Flags: review?(smichaud)

    
    

    
  
Comment on attachment 664447 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Fixed various popup-related issues seen with the previous version, and also cleaned up some layering violations. Tryserver build should be at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-4f0f43fb7972 in a couple hours.

        Attachment #664447 -
        Flags: review?(roc)

    
  
Blocks: 794038

    
    

    
  
In this bug, I'm proposing that we should land enough patches to enable HiDPI rendering on retina-display systems with a single screen. HiDPI mode will initially be preffed-off on configurations where there are multiple screens with differing backing resolutions.

I've filed bug 794038 as a followup to handle the multi-screen issues, as this bug is plenty long enough already.

    
    

    
  
Comment on attachment 664447 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Review of attachment 664447 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

::: layout/xul/base/src/nsXULPopupManager.cpp
@@ +318,5 @@
>  
> +  // Convert desired point to CSS pixels for comparison
> +  nsPresContext* presContext = menuPopupFrame->PresContext();
> +  aPnt.x = presContext->DevPixelsToIntCSSPixels(aPnt.x);
> +  aPnt.y = presContext->DevPixelsToIntCSSPixels(aPnt.y);

Can you add comments to nsMenuPopupFrame::ScreenPosition and mXPos/mYPos/mScreenXPos/mScreenYPos that these are all in CSS pixels?

::: widget/cocoa/nsChildView.h
@@ +410,5 @@
>    virtual bool            IsEnabled() const;
>    NS_IMETHOD              SetFocus(bool aRaise);
>    NS_IMETHOD              GetBounds(nsIntRect &aRect);
>  
> +  CGFloat                 BackingScaleFactor();

Document this.

::: widget/cocoa/nsCocoaUtils.mm
@@ +40,5 @@
> +    NSRect frame = [screen frame];
> +    CGFloat scale = 1.0;
> +    if (nsCocoaUtils::HiDPIEnabled() &&
> +        [screen respondsToSelector:@selector(backingScaleFactor)]) {
> +      scale = [screen backingScaleFactor];

So this is where you're depending on the screens all having the same backingScaleFactor?

Might as well document these places with a comment.

::: widget/cocoa/nsCocoaWindow.mm
@@ +220,5 @@
>    return false;
>  #endif /* MOZ_USE_NATIVE_POPUP_WINDOWS */
>  }
>  
> +// aRect here is specified in CSS pixels

Er, really? I would have expected this to be device pixels currently. But maybe it makes sense to make it CSS pixels since we don't know what screen it's on. OK!

If we're going to make this CSS pixels, then we should document that in nsIWidget.h

@@ +1349,5 @@
>    NSRect frame = NSZeroRect;
> +  if (mWindow) {
> +    frame = nsCocoaUtils::CocoaPointsToDevPixels([mWindow frame], BackingScaleFactor());
> +  }
> +  mBounds = nsCocoaUtils::CocoaRectToGeckoRectDevPix(frame);

Wouldn't it make sense for nsCocoaUtils::CocoaRectToGeckoRectDevPix to take BackingScaleFactor() as a parameter and always convert from Cocoa points?

@@ +1642,4 @@
>      rect = [mWindow contentRectForFrameRect:[mWindow frame]];
> +    rect = nsCocoaUtils::CocoaPointsToDevPixels(rect, BackingScaleFactor());
> +  }
> +  r = nsCocoaUtils::CocoaRectToGeckoRectDevPix(rect);

ditto

::: widget/cocoa/nsDragService.mm
@@ +262,5 @@
>    }
>  
> +  NSPoint point = NSMakePoint(dragRect.x, dragRect.YMost());
> +  if ([gLastDragView respondsToSelector:@selector(backingScaleFactor)]) {
> +    CGFloat scale = [gLastDragView backingScaleFactor];

Maybe nsCocoaUtils should have a method which calls respondsToSelector and calls backingScaleFactor or returns 1.0? You do this all over the place.

    
    

    
  
The latest tryserver build (comment #297) has problems with multiple screens, even if they are not concurrently used. Switching screens is broken (was also broken in build #281, was *not* in build #229): 
If I launch Firefox in clamshell mode with external 1920x1200 screen, and then remove the rMBP from the external display, then when I open the lid I get Firefox windows rendered at native resolution: the lower left quadrant is occupied by the window content, and garbage fills the rest. The controls (tabs etc) are positioned correctly, so that I can switch tabs by clicking "phantom tabs", and this correctly refreshes the half-scale image in the bottom left.
Basically Firefox works, but doesn't seem to register the new screen config (I'm using 1680x1050 reoslution on the retina screen, BTW).

    
    

    
  
Worth testing that various DOM properties tested by http://mpulp.mobi/labs/ppk/widthtest.html return CSS pixels and don’t return device pixels.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #298)
> In this bug, I'm proposing that we should land enough patches to enable
> HiDPI rendering on retina-display systems with a single screen. HiDPI mode
> will initially be preffed-off on configurations where there are multiple
> screens with differing backing resolutions.
> 
> I've filed bug 794038 as a followup to handle the multi-screen issues, as
> this bug is plenty long enough already.

This is exactly right. Thank you Jonathan.

    
    

    
  
Seems like the latest tryserver build regressed on how it renders popups:

1. http://cl.ly/image/1h0w0o3P1w1Y
2. http://cl.ly/image/3U3Y2b2t1q3R
3. http://cl.ly/image/2M063q1i363Y

Apologies if these are known (expected?) issues.

    
    

    
  
Sorry, yes, that build was broken - I messed up a last-minute adjustment to the patch, and must've failed to update my local build properly. I'm about to push a new version that I trust will fix these problems.

    
    

    
  


  

        Attachment #664587 -
        Flags: review?(smichaud)

        Attachment #664587 -
        Flags: review?(roc)

    
  

        Attachment #664447 -
        Attachment is obsolete: true

        Attachment #664447 -
        Flags: review?(smichaud)

        Attachment #664447 -
        Flags: review?(roc)

    
    

    
  
Actually I discovered that the change to half-scale rendering could happen even on the 'old' 21/08 build.
Start Firefox on external display (in clamshell mode), unplug, open lid, and now try to zoom in a window: this *sometimes* switches to 1/2 scale mini-windows (they were in top left quadrant this time, not too sure whether it's a question of build version or if it's random). I don't know either what triggers the 'jivaro reduction' of these pages; if you've a suspect I can check.
Of course reloading the page cures all (if you happen to be connected).

    
    

    
  
Jonathan, the following build failure probably indicates that you need to add another definition of -(CGFloat)backingScaleFactor;, and possibly that you also need to cast an NSObject* to NSView before calling that method on it.

https://tbpl.mozilla.org/php/getParsedLog.php?id=15522770&tree=Try#error0

    
    

    
  
(In reply to comment #307)

Pierre, we already know that current patches don't support dynamic switching between HiDPI and non-HiDPI modes.  That problem's been spun off to bug 794038.

    
    

    
  
OK I misread bug 794038 as only concerning multi-monitor displays.
Nevertheless, and crucially, the 21/08 build provided some dynamic switching (up to this zooming error, windows were correctly rendered), whereas the newest builds do not (half-scale). 
Sorry if you were already aware of this (didn't see it reported).

    
    

    
  


  

        Attachment #664990 -
        Flags: review?(smichaud)

    
  

        Attachment #664587 -
        Attachment is obsolete: true

        Attachment #664587 -
        Flags: review?(smichaud)

    
    

    
  
Jonathan, I just noticed a serious problem testing with your latest patch:

On some web pages, everything on the page is half the size it should be.  Here's an example:

http://mirrors.creativecommons.org/

I don't believe this happened with earlier versions of your patch.  I'll check, though.

I'll also try your very latest patch :-)

    
    

    
  
Tryserver build with latest patch here is available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-a3fe76fbe386. (This fixes the build problem noted in comment 308).

I'm not seeing the half-size problem you describe, with either my local debug build or the tryserver opt build. It sounds like what could happen if you opened the page on a non-HiDPI display and then moved the window to a HiDPI one, but it shouldn't happen in a straightforward single-display situation.

Carrying forward r=roc for the cocoa patch (pt 6), but I'd appreciate you looking it over as well, obviously.

    
    

    
  
> It sounds like what could happen if you opened the page on a non-HiDPI display and
> then moved the window to a HiDPI one

That's not what I did.  I tested on a Retina MBP with no attached external display.  I'll get to the bottom of this during the review process (presuming I can reproduce it with your latest patch).

Also note that all the chrome was the right size.  Only the page contents had the wrong size, and this didn't happen with another page.

    
    

    
  
(Following up comment #314)

Nevermind.  This bug also happens in today's mozilla-central nightly, so it has nothing to do with your patch.

(And sorry for breaking my own rule about first checking the nightlies.)

    
    

    
  
(Following up comment #315)

There's no bug after all.  I forgot that I'd previously zoomed that page down, and just learned that zoom settings are preserved across Firefox sessions.

Sorry for the confusion :-(

    
    

    
  
I've also pushed a tryserver job based on today's mozilla-central tree with the latest patches here, plus your plugin patch from bug 785667 comment 68; results should appear at https://tbpl.mozilla.org/?tree=Try&rev=c72f3b9f3ef4.

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #317)
> I've also pushed a tryserver job based on today's mozilla-central tree with
> the latest patches here, plus your plugin patch from bug 785667 comment 68;
> results should appear at https://tbpl.mozilla.org/?tree=Try&rev=c72f3b9f3ef4.

Looks like the OS X build is coming up as 'BUSTED'

    
    

    
  
(In reply to Jonathan Guerin from comment #318)
> (In reply to Jonathan Kew (:jfkthame) from comment #317)
> > I've also pushed a tryserver job based on today's mozilla-central tree with
> > the latest patches here, plus your plugin patch from bug 785667 comment 68;
> > results should appear at https://tbpl.mozilla.org/?tree=Try&rev=c72f3b9f3ef4.
> 
> Looks like the OS X build is coming up as 'BUSTED'

Scratch that, I misread the build output. Running it now, plugins appear to run well, drop-down issue is fixed. Will keep running it and let you know.

    
    

    
  
This build still has issues with YouTube video playback controls, whereas Steven Michaud's HiDPI build from bug 785667 does not.

    
    

    
  
(In reply to Antonio from comment #320)
> This build still has issues with YouTube video playback controls, whereas
> Steven Michaud's HiDPI build from bug 785667 does not.

I'm seeing Flash YouTube videos working fine, but HTML5 ones are jerking badly.

    
    

    
  
This latest build is also showing a bunch of failures in test_media_queries.html, on both OS X and Android (other platforms not yet completed). The build from comment 313 didn't suffer from that; not yet sure what's causing it. Note that I rebased my tree between comment 313 and the latest build, which may have introduced a new issue.

    
    

    
  
Comment on attachment 664990 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

This looks fine to me, though I have a few nits:

>   if (!painted && [self isOpaque]) {
>     // Gecko refused to draw, but we've claimed to be opaque, so we have to
>     // draw something--fill with white.
>     CGContextSetRGBFillColor(aContext, 1, 1, 1, 1);
>-    CGContextFillRect(aContext, CGRectMake(aRect.origin.x, aRect.origin.y,
>-                                           aRect.size.width,
>-                                           aRect.size.height));
>+    CGContextFillRect(aContext, CGRectMake(boundingRect.x, boundingRect.y,
>+                                           boundingRect.width,
>+                                           boundingRect.height));
>   }

>   CGContextSetRGBStrokeColor(aContext, 1, 0, 0, 0.8);
>   CGContextSetLineWidth(aContext, 4.0);
>   CGContextStrokeRect(aContext,
>-                      CGRectMake(aRect.origin.x, aRect.origin.y,
>-                                 aRect.size.width, aRect.size.height));
>+                      CGRectMake(boundingRect.x, boundingRect.y,
>+                                 boundingRect.width, boundingRect.height));

I suspect these two parts of the patch should be reversed.  As you say in a prior comment, the CGContext passed to [ChildView drawRect:inContext:] is scaled to "cocoa points".

>   /**
>    * Returns NSRect for aGeckoRect.
>    */
>   static void GeckoRectToNSRect(const nsIntRect& aGeckoRect,
>                                        NSRect& aOutCocoaRect);
 
>   /**
>+   * Returns Gecko rect for aCocoaRect.
>+   */
>+  static void NSRectToGeckoRect(const NSRect& aCocoaRect,
>+                                nsIntRect& aOutGeckoRect);
>+

Both of these should have comments saying they only work with rectangles whose coordinate systems have the same origin (e.g. both top-left or both bottom-left).  (Yes, this is a drive-by.  But it's bugged me for a while.)

Have you checked that the following two calls to nsCocoaUtils::GeckoRectToCocoaRect() are called with geckoRect in "cocoa points"?

http://hg.mozilla.org/mozilla-central/annotate/a425ea4f16c9/accessible/src/mac/mozTextAccessible.mm#l220
http://hg.mozilla.org/mozilla-central/annotate/a425ea4f16c9/widget/cocoa/nsScreenManagerCocoa.mm#l45

        Attachment #664990 -
        Flags: review?(smichaud) → review+

    
    

    
  
Oh, I see... the hidpi-plugins patch made a change to test_media_queries.html, but that needs to be revised to account for the zoomRatio -> fullZoom change that Roc asked for in comment 294. That explains all those mochitest failures on the try build.

    
    

    
  
(In reply to comment #324)

The odd thing is that that I didn't see these test failures before today (that change to test_media_queries.html has been in my patches for about a week).  And my change seems correct (it matches changes you made to other tests, where you replaced call to nsIDOMWindowUtils.screenPixelsPerCSSPixel with calls to nsIDOMWindowUtils.zoomRation).

I don't see zoomRatio -> fullZoom anywhere in your patches.

    
    

    
  
Ah, sorry, it's a change I made locally in response to review comments on part 5. I'll upload the modified version here for the record.

    
  

        Attachment #663478 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 665130 [details] [diff] [review]
pt 5 - provide a fullZoom API in nsIDOMWindowUtils, and use this rather than inferring zoom from CSS to device pixel ratio.

Changed zoomRatio to fullZoom; carrying forward r=roc from previous version in comment 294.

        Attachment #665130 -
        Flags: review+

    
    

    
  
Jonathan, do you mind if I simply drop my patch's change to /layout/style/test/test_media_queries.html, and let you deal with any possible consequences?

It's not related to the rest of my patch.  I just made that change because it matched other changes you'd made, and I thought you'd overlooked that test.

    
    

    
  
That's fine. The tests pass as they stand; I'll try to check whether it's more correct to make the change, but it's not necessary at this point, at least.

    
    

    
  
I'm currently doing local builds of my patch without changes to the test_media_queries.html test (plus your very latest patch).  I'll first check locally for failures in that test, then on the tryserver.

It *should* fail without any changes, and I think it once would have.  But I suspect something happened (elsewhere in the tree) to stop those changes happening.

    
    

    
  
No, that test really does want to use the screen-pixels-per-CSS-pixel ratio, not the page zoom ratio, so changing the DOMWindowUtils API it's using is wrong. But we should rename the test's helper function getZoomRatio() as getScreenPixelsPerCSSPixel() instead, to avoid the confusion.

(It wouldn't have failed on tryserver either way, because tryserver doesn't run in HiDPI configurations, but local testing confirms that screenPixelsPerCSSPixel is the right value to use for the -moz-device-pixel-ratio media-query tests.)

    
  

        Attachment #664990 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 665155 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Minor updates for review comments above; carrying forward r=roc,smichaud.

        Attachment #665155 -
        Flags: review+

    
    

    
  
(In reply to comment #332)

Now I see that you're right.
No longer blocks: 784909

    
  

        Attachment #665155 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 665318 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Minor update to un-break the multi-screen detection, which was accidentally broken due to misuse of nsCocoaUtils::GetBackingScaleFactor(), and tidy up coord conversions in nsCocoaUtils a bit. This results in a more reasonable origin for the "device pixels" coordinate space on secondary screens, which will help in bug 794038. Carrying forward r=roc,smichaud.

        Attachment #665318 -
        Flags: review+

    
    

    
  
And a tryserver build with these patches plus Steven's latest plugins patch (v1.6) from bug 785667 is in progress at https://tbpl.mozilla.org/?tree=Try&rev=ba47347cb2b1.

    
  

        Attachment #665318 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 665418 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Another minor fix, correcting the handling of size constraints on cocoa windows. Carrying forward reviews again.

        Attachment #665418 -
        Flags: review+

    
    

    
  
Just built all the latest patches + fryn's UI patch and it looks great! :)

Last remaining obvious bug I am encountering is that when dragging a tab the thumbnail image is offset by some amount and that amount increase the more you drag tabs:

http://cl.ly/image/3p333c081j16

    
    

    
  
(In reply to Stephen Horlander from comment #341)
> Just built all the latest patches + fryn's UI patch and it looks great! :)
> 
> Last remaining obvious bug I am encountering is that when dragging a tab the
> thumbnail image is offset by some amount and that amount increase the more
> you drag tabs:
> 
> http://cl.ly/image/3p333c081j16

Build is also working great for me. Plugins seem good, and drop-downs are fixed. Also seem to be seeing less crashes lately. ;)

    
    

    
  
On the newest try build (not on current nightly) I get a blank black screen when I tear off a tab to create a new window.

Steps to reproduce:
1. Open multiple tabs
2. select tab; tear off from window group
3. new window appears

What is expected:
tab is opened as displayed in new window

what actually happens:
normal header is displayed above where tab bar should be;
everything else in the window is black.

System:
Retina Mackbook Pro Mid 2012
No external displays or windows

Possibly related: bug #794038

    
    

    
  
Hmm, yes. If you need a workaround for now, just adjust the size of the new window slightly - even a pixel or two will do - and its content should snap into view.

    
  

        Attachment #665418 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 665857 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

This fixes the problem of getting a completely-black window when tearing off a tab. Window initialization happens a little differently in this scenario from the normal "New Window" command, and I'd missed a coordinate conversion that's  important for this case.

Tryserver build in progress at https://tbpl.mozilla.org/?tree=Try&rev=73d8c7863404. Carrying forward r=roc,smichaud again.

        Attachment #665857 -
        Flags: review+

    
    

    
  
(In reply to Stephen Horlander from comment #341)
> Just built all the latest patches + fryn's UI patch and it looks great! :)
> 
> Last remaining obvious bug I am encountering is that when dragging a tab the
> thumbnail image is offset by some amount and that amount increase the more
> you drag tabs:
> 
> http://cl.ly/image/3p333c081j16

Yes, I've been seeing that problem, but haven't figured out exactly where best to fix it yet. I'd be prepared to land these patches as-is and handle that as a followup, if necessary, as it's a relatively cosmetic issue rather than a crippling deficiency.

    
  

        Attachment #665857 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 665871 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

And here's an update that also fixes the tab-drag-image problem (note that it was only half the size it should have been, as well as being offset).

Tryserver build in progress at https://tbpl.mozilla.org/?tree=Try&rev=aab7a1bee46e.

        Attachment #665871 -
        Flags: review+

    
    

    
  
Comment on attachment 665871 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -3572,22 +3572,25 @@
>         // otherwise trying to deatch the tab by dropping it on the desktop
>         // may result in an "internet shortcut"
>         dt.mozSetDataAt("text/x-moz-text-internal", browser.currentURI.spec, 0);
> 
>         // Set the cursor to an arrow during tab drags.
>         dt.mozCursor = "default";
> 
>         // Create a canvas to which we capture the current tab.
>+        let windowUtils = browser.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor).
>+                          getInterface(Ci.nsIDOMWindowUtils);
>+        let scale = windowUtils.screenPixelsPerCSSPixel / windowUtils.fullZoom;
>         let canvas = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas");
>         canvas.mozOpaque = true;
>-        canvas.width = 160;
>-        canvas.height = 90;
>+        canvas.width = 160 * scale;
>+        canvas.height = 90 * scale;
>         PageThumbs.captureToCanvas(browser.contentWindow, canvas);
>-        dt.setDragImage(canvas, -16, -16);
>+        dt.setDragImage(canvas, -16 * scale, -16 * scale);

Why is this needed? canvas.width and canvas.height should be CSS pixels, right? I'd expect this to just work.

    
    

    
  
Comment on attachment 665871 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Steven, mind re-reviewing this for the most recent fixes, particularly the drag-image stuff? Thanks!

        Attachment #665871 -
        Flags: review+ → review?(smichaud)

    
    

    
  
(In reply to Dão Gottwald [:dao] from comment #350)

> Why is this needed? canvas.width and canvas.height should be CSS pixels,
> right? I'd expect this to just work.

Canvas doesn't know about hidpi yet, so that results in a canvas that is 160x90 pixels, which then gives us a drag image only half the size we want; we could then render it as 160x90 CSS pixels, but the result would be a low-res upscaled image instead of a full-resolution one, which looks much better.

Once we make canvas hidpi-aware, we'd be able to undo this and just let the canvas draw to a hidpi surface internally, but that's some way off yet (bug 780362).

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #352)
> (In reply to Dão Gottwald [:dao] from comment #350)
> 
> > Why is this needed? canvas.width and canvas.height should be CSS pixels,
> > right? I'd expect this to just work.
> 
> Canvas doesn't know about hidpi yet, so that results in a canvas that is
> 160x90 pixels, which then gives us a drag image only half the size we want;
> we could then render it as 160x90 CSS pixels, but the result would be a
> low-res upscaled image instead of a full-resolution one, which looks much
> better.
> 
> Once we make canvas hidpi-aware, we'd be able to undo this and just let the
> canvas draw to a hidpi surface internally, but that's some way off yet (bug
> 780362).

Can you document this in a code comment, including the bug reference?

    
    

    
  
Comment on attachment 665871 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -3572,22 +3572,25 @@
>         // otherwise trying to deatch the tab by dropping it on the desktop
>         // may result in an "internet shortcut"
>         dt.mozSetDataAt("text/x-moz-text-internal", browser.currentURI.spec, 0);
> 
>         // Set the cursor to an arrow during tab drags.
>         dt.mozCursor = "default";
> 
>         // Create a canvas to which we capture the current tab.
>+        let windowUtils = browser.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor).
>+                          getInterface(Ci.nsIDOMWindowUtils);

Is there any reason for using the content window rather than the chrome window here? This looks like it could just be window.getInterface(Ci.nsIDOMWindowUtils).

    
  

        Attachment #665871 -
        Attachment is obsolete: true

        Attachment #665871 -
        Flags: review?(smichaud)

    
    

    
  
(In reply to Dão Gottwald [:dao] from comment #353)
> Can you document this in a code comment, including the bug reference?

Done.

(In reply to Dão Gottwald [:dao] from comment #354)
> This looks like it could just be
> window.getInterface(Ci.nsIDOMWindowUtils).

Indeed it could, thanks.

    
    

    
  
Comment on attachment 665896 [details] [diff] [review]
pt 6 - support HiDPI display in Cocoa widget code.

Looks fine to me.

Good catch on your change to nsChildView::Create().  I should have caught that myself in my previous review.

        Attachment #665896 -
        Flags: review?(smichaud) → review+

    
    

    
  
Tooltips don't seem to be working properly for me with these patches applied. I'm not certain yet that one of these patches is at fault, but I'm noting it here just in case.
Status: NEW → ASSIGNED

    
    

    
  
There's an intermittent issue with tooltips as described in comment #160 and following, which I'm still seeing in current builds (even though we're now using a very different set of patches!), but it seems relatively minor.

Could you be more specific about how they're not working for you: are they not displayed at all / garbled / wrong size / in the wrong place /....?

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #359)
> There's an intermittent issue with tooltips as described in comment #160 and
> following, which I'm still seeing in current builds (even though we're now
> using a very different set of patches!), but it seems relatively minor.

I had noticed this intermittent issue before, but I no longer see it in the current builds.

> Could you be more specific about how they're not working for you: are they
> not displayed at all / garbled / wrong size / in the wrong place /....?

At some point (for which I don't yet have reliable STR), it gets stuck displaying one particular string in the tooltip, i.e. whenever the tooltip is shown while the cursor is inside the content area, it displays that one string.

I have been using mozilla-inbound + the patches in this bug + HiDPI plugins patch + 2x browser UI images as my primary browser for several weeks now. With all the latest versions of these patches, it's working rather well, and even with the glitches it's definitely an improved experience. :)

    
    

    
  
(In reply to Frank Yan (:fryn) from comment #360)
> At some point (for which I don't yet have reliable STR), it gets stuck
> displaying one particular string in the tooltip, i.e. whenever the tooltip
> is shown while the cursor is inside the content area, it displays that one
> string.

bug 795576 (DLBI fallout)

    
    

    
  
Pushed to inbound (parts 1-6):
https://hg.mozilla.org/integration/mozilla-inbound/rev/66bc6ceca2f3
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d715c595838
https://hg.mozilla.org/integration/mozilla-inbound/rev/466d49964ff1
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e8929729b4c
https://hg.mozilla.org/integration/mozilla-inbound/rev/550641381dfa
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d3de8da2508

This should enable HiDPI rendering on retina macbooks, except when a non-HiDPI display is also attached (see bug 794038 for multi-/mixed-resolution issues).

Note that until bug 785667 lands, HiDPI users will experience some issues with plugins: in some cases, plugin content will be incorrectly scaled, and/or interaction with controls will be broken.

If you need to work around this until plugin support lands, go to about:config and set "gfx.hidpi.enabled" to 0 (and restart the browser) to revert to non-HiDPI rendering.

Unless this needs to be backed out for unexpected failures, please *file new bugs* for any remaining glitches in HiDPI rendering, rather than adding to this bug, and mark them as blocking the HiDPI tracker (bug 785330).
Target Milestone: --- → mozilla18

    
    

    
  
(In reply to Jonathan Kew (:jfkthame) from comment #362) 
> This should enable HiDPI rendering on retina macbooks, except when a
> non-HiDPI display is also attached (see bug 794038 for
> multi-/mixed-resolution issues).

Johnathan, Steven, Robert, and all those who helped test this work, thank you for your efforts. This is a major step forward for our Mac support.

    
    

    
  
Allow me to express my gratitude (as a user) to everyone who worked on this bug!  Fantastic leap ahead in terms of usability.  I can finally get off the try server builds and start using the nightly again.

    
    

    
  
Big thanks to all you guys -- regular nightly build now looks great on my Retina MacBook Pro, and it even 'just works' with the code we're working on for supporting high-DPI images on Wikipedia!

    
    

    
  
Using the latest Nightly (last night's) on my rMBP running Linux (Ubuntu 12.10 to be specific), Firefox still looks less than awesome: the text is tiny, images aren't scaled appropriately, etc.

I can't find any bugs which indicate that retina-type high DPI support is being worked on for any other platforms in Firefox than OS X. Is such work being done?

    
    

    
  
(In reply to Craig from comment #367)
> Using the latest Nightly (last night's) on my rMBP running Linux (Ubuntu
> 12.10 to be specific), Firefox still looks less than awesome: the text is
> tiny, images aren't scaled appropriately, etc.
> 
> I can't find any bugs which indicate that retina-type high DPI support is
> being worked on for any other platforms in Firefox than OS X. Is such work
> being done?

On Linux or Windows, you'll want to go to about:config and switch the option "layout.css.devPixelsPerPx" manually to 2.

These OSs don't have a thorough infrastructure for mixed high- and low-DPI screen setups as OS X does, but if you crank the resolution up there are various application-specific ways to increase font sizes to match. (For instance you've probably already discovered that you can scale up many things in GNOME apps by adjusting the font size scale in the optional "advanced" settings panel, but this only affects the menus and such in Firefox.)

If you think this setting should get engaged automatically on Linux or Windows when the font scaling setting is high, go ahead and open a new bug specifically for that.

OS X is sort of its own special beast and implements high-DPI rendering in a manner more similar to how iOS and Android do, using a scale factor and a combination of device-independent and device-dependent units, so had to get handled specially for this bug.

    
    

    
  
Bug 603880 is the Windows bug. I'm not sure if we have a similar bug filed for Linux/GTK.

    
    

    
  

      
      
      
      

        Attached image
          
        Bad window rendering
        — 
      

    


  
Using the latest nightly, 18.0a1 (2012-10-03):

I had an external monitor plugged into my retina MBP. I unplugged the monitor and then put my laptop to sleep. The firefox windows looked ok. I then woke the laptop with no external displays and (I think) the windows still looked ok, but they were not in the foreground. I Cmd-Tab'd Firefox into the foreground and all of its windows look like the attached image.

    
    

    
  
Comment on attachment 667530 [details]
Bad window rendering

Note: I'll try to reproduce this when I get home using a clean profile. Although, I wonder if it could be related to a bug in OSX. I have noticed that sometimes if I plug in my external display followed by a USB2.0 hub containing a wireless mouse adapter that my retina display (but not the external) will draw weird blocks as I move the mouse over it. Unplugging and re-plugging the external monitor fixes the problem.

    
    

    
  
(In reply to Harvey Chapman from comment #370)
> I had an external monitor plugged into my retina MBP. I unplugged the
> monitor and then put my laptop to sleep. The firefox windows looked ok. I
> then woke the laptop with no external displays and (I think) the windows
> still looked ok, but they were not in the foreground. I Cmd-Tab'd Firefox
> into the foreground and all of its windows look like the attached image.

The current code is known not to handle this case well; multi-monitor configurations and dynamically changing resolution are being covered in the split-off bug 794038.

It's a bit ugly, but for now you'll have to close and restart Firefox nightly after switching monitors. :(

    
    

    
  
(In reply to Brion Vibber from comment #372)
> The current code is known not to handle this case well; multi-monitor
> configurations and dynamically changing resolution are being covered in the
> split-off bug 794038.
> 
> It's a bit ugly, but for now you'll have to close and restart Firefox
> nightly after switching monitors. :(

Ah, thanks for the info. I'll follow the new bug. I can live with the workaround since restarting is no longer the problem it used to be, i.e. Session Manager and delayed loading of background tabs.

    
    

    
  
While having to reload after a monitor switch may be ugly the inconvenience is barely noticeable when you factor in HiDPI works(!!!) and if you have Sync configured.

Awesome work, all.

    
    

    
  
Early in this thread (In reply to Steven Michaud from comment #49)
> > If I go into about:config and set "layers.acceleration.disabled" to
> > true, then text looks beautifully high-resolution, as do the
> > thumbnails on the new tab page.
> 
> Very interesting.  Thanks for finding this out, and letting us know. 
> 
> We can't turn off layers acceleration.  But knowing that this setting
> makes a difference gives us a much better idea where we'll need to
> make changes.

Why is layers acceleration so important that it can't (temporarily) be turned off?

I've been running that nightly build of v16 with layers acceleration disabled since I got my retina macbook, and it works perfectly. The new nightlys - which I've tried - continue to have huge issues with plugins and the download bar.

Why is it not possible to turn that build into a temporary Retina-compatible build? Is there a way to get nightly to not automatically update?

The alternative is to have every retina user - and the press - see Firefox as an unusable laggard. Chrome fixed this fast. Even Microsoft, which has always been a slow and arguably terrible mac developer, released a retina update to its office suite recently.

This may not be the best place for this comment, but I thought it's worth at least flagging.
Depends on: 800668

    
  
Depends on: 800948

    
  
Depends on: 804644

    
  
Depends on: 806059
Depends on: 811667
Depends on: 815512

    
  
No longer depends on: 815512
Depends on: 825734

    
  
Depends on: 830731
Depends on: 840878

    
  
Depends on: 799523
Depends on: 878098

    
    

    
  
I notice that there are still aspects in Firefox 27.0 that are not retina. Mainly icons.

- all bookmark related icons, i.e. folder, default link icons, and "Get Bookmark Add-ons" icon.
- spinner when checking for new updates.
- All "Preferences" icons.
- All icons in the Add-ons manager.

I guess it's just a question of making the right icons available since Firefox 27.0 is largely Retina-ready.

    
    

    
  
(In reply to sub from comment #377)
> I notice that there are still aspects in Firefox 27.0 that are not retina.
> Mainly icons.
> 
See Bug 854956

    
    

    
  
(In reply to José Jeria from comment #378)
> (In reply to sub from comment #377)
> > I notice that there are still aspects in Firefox 27.0 that are not retina.
> > Mainly icons.
> > 
> See Bug 854956

Right... Bug 854956 deals with high-res favicons, but what I was referring to was other "native" icons in Firefox.

The following in the bookmarks dropdown menu:
- folder icons for bookmarks (including smart folder icons)
- link icons for links without favicons 
- "Bookmarks toolbar" icon
- RSS folder icon
- the "get bookmark add-ons" icon

All of the above in the "Show All Bookmarks" pane as well as all the Toolbar icons in the bookmarks pane.

Icons in the Preferences Pane:
- All top icons
- (?) help button
- the action icons under Applications

All icons in the Add-ons manager

I suppose it is related to Bug 854956 in that some of the same bookmark icons are high-res if added to the Bookmarks Toolbar so the high-res images are there (presumably due to Bug 854956 fixes) but they are not implemented for the Bookmarks dropdown menu or Bookmarks pane.

The lack of high-res icons goes further than that though since it also an issue with the Preferences Pane, Add-ons manager and toolbar icons in the Bookmarks Pane.

    
    

    
  
(In reply to chris from comment #379)
> (In reply to José Jeria from comment #378)
> > (In reply to sub from comment #377)
> > > I notice that there are still aspects in Firefox 27.0 that are not retina.
> > > Mainly icons.
> > > 
> > See Bug 854956
> 
> Right... Bug 854956 deals with high-res favicons, but what I was referring
> to was other "native" icons in Firefox.
> 
> The following in the bookmarks dropdown menu:
> - folder icons for bookmarks (including smart folder icons)
> - link icons for links without favicons 
> - "Bookmarks toolbar" icon
> - RSS folder icon
> - the "get bookmark add-ons" icon
> 
> All of the above in the "Show All Bookmarks" pane as well as all the Toolbar
> icons in the bookmarks pane.
> 
> Icons in the Preferences Pane:
> - All top icons
> - (?) help button
> - the action icons under Applications
> 
> All icons in the Add-ons manager
> 
> I suppose it is related to Bug 854956 in that some of the same bookmark
> icons are high-res if added to the Bookmarks Toolbar so the high-res images
> are there (presumably due to Bug 854956 fixes) but they are not implemented
> for the Bookmarks dropdown menu or Bookmarks pane.
> 
> The lack of high-res icons goes further than that though since it also an
> issue with the Preferences Pane, Add-ons manager and toolbar icons in the
> Bookmarks Pane.

I should add the embedded window that appears for activating plugins to the list above too. In particular the cross for closing it.

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: