Closed Bug 1573157 Opened 1 year ago Closed 11 months ago

Load macOS icons with dual representations

Categories

(Core :: Widget: Cocoa, enhancement)

enhancement
Not set
normal
Points:
1

Tracking

()

RESOLVED FIXED
mozilla71
Iteration:
70.4 - Aug 19 - Sep 1
Tracking Status
firefox71 --- fixed

People

(Reporter: harry, Assigned: harry)

Details

Attachments

(1 file)

Currently, some icons on macOS load with nsCocoaUtils::CreateNSImageFromImageContainer, which takes a CGFloat scaleFactor parameter. The work in bug 1465403 introduced nsCocoaUtils::CreateDualRepresentationNSImageFromImageContainer which loads an NSImage* with both 1x and 2x representations. We should migrate our macOS icon loading code to use this new method to better support users moving between HiDPI and regular displays (for instance, a Retina MacBook connected to a 1x external monitor).

There are two places in-tree that use nsCocoaUtils::CreateNSImageFromImageContainer: loading cursors, and loading images in Notification Center. Looking into this more, I think only the notification center call needs to be changed. The cursor is constantly reset as it moves, calling BackingScaleFactor() to determine if a 1x or 2x cursor should be used. This is preferable to using CreateDualRepresentationNSImageFromImageContainer since the mouse will only be on one screen at any given time.

As best I can tell, Notification Center forces loading images @1x only because CreateNSImageFromImageContainer added a scaleFactor parameter right before the notification center patch was about to land, so forcing 1.0f scaling was just a stopgap taken to avoid refactoring that patch. That is, CreateDualRepresentationNSImageFromImageContainer is a better choice there.

Assignee: nobody → htwyford
Status: NEW → ASSIGNED
Iteration: --- → 70.4 - Aug 19 - Sep 1
Points: --- → 1
Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2541bc7ccddd
Load notification center icons with dual representations. r=mstange
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.