Closed Bug 1096132 Opened 10 years ago Closed 10 years ago

clarify use of "scale" in nsDeviceContext

Categories

(Core :: Layout, defect)

36 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: karlt, Unassigned)

Details

Attachments

(3 files, 1 obsolete file)

The concept of UnscaledAppUnitsPerDevPixel() and mAppUnitsPerDevNotScaledPixel
for page zoom dates back to implementation in Bug 4821.

Now that we have a GetDefaultScale, which means something quite different, it
would be helpful to distinguish the "scale" from the page full zoom identifiers.
I'm still not entirely happy with the name here, but I think it is an
improvement to remove any suggestion that this is related to not applying the
window "DefaultScale".

AIUI the reason for not applying zoom is that screen coordinates do not change
under page full zoom.
Attachment #8519660 - Flags: review?(tnikkel)
I would like it if we also made it harder to confuse the concepts of the desktop "full zoom" from what we do on b2g and android, the "pinch zoom". Although "full zoom" isn't a great term, it's better than nothing.

Do you think we could put "full" everywhere you are renaming to zoom here? I'm also open to better suggestions. Only the last patch might be hard, not sure how to say un-"full zoom"'ed.
I thought I might be able to get away with just Zoom on/in the device context because it is the device context zoom, but I'm happy to add "Full" or "Page", whichever you prefer, if that helps.

Re ConvertToUnscaledDevPixels, the input is screen coords but not ScreenPixels.
These are often called display pixels in widget code, though that is not a natural term to me.  They are a kind of device-independent pixel and are often called CSS Pixels.  It is really multiplying by GetWindowScale() but we have an nsPresContext instead of an nsIWidget.

Possible options:

ConvertDisplayPixelsToDevPixels
ConvertScreenCoordsToDevPixels
ConvertCSSPixelsToDevPixels
So ConvertToUnscaledDevPixels is applying the window "default" scale, and perhaps the name is OKish as it is.  It just feels like a weird way of saying it to me.  It is really ScaleToDevPixels.
ConvertToUnitFullZoomDevPixels?
Comment on attachment 8519660 [details] [diff] [review]
rename ConvertToUnscaledDevPixels() to ConvertToUnzoomedDevPixels()

Much of the widget code is confused about what display pixels are, and view
code uses the term for two different things.
Most code seems to assume display pixels are Gecko CSS/HTML/DOM screen units,
which are related to device pixels by nsIWidget::GetDefaultScale().

Some code assumes display pixels are the OS device-independent pixels, which
are related to device pixels by Cocoa's backingScaleFactor, or NT's dpi ratio.
Much is this is probably buggy, but nsView::CalcWidgetBounds()'s use of
nsIWidget::RoundsWidgetCoordinatesTo() looks correct.

I also see that nsIBaseWindow.idl has unscaledDevicePixelsPerCSSPixel (which is GetDefaultScale()) and I'm not wanting to change a scriptable API.  ConvertToUnscaledDevPixels() is consistent with that, so I think I'll leave that as is, and focus on nsDeviceContext in this bug.
Attachment #8519660 - Attachment is obsolete: true
Attachment #8519660 - Flags: review?(tnikkel)
Attachment #8519655 - Flags: review?(tnikkel) → review+
Attachment #8519656 - Flags: review?(tnikkel) → review+
Attachment #8519657 - Flags: review?(tnikkel) → review+
You need to log in before you can comment on or make changes to this bug.