1099314 - Rename nsDisplayImage::GetDestRect, to clear up ambiguity between it & the "aDest" rect in image drawing functions

Status: RESOLVED WORKSFORME

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Daniel Holbert [:dholbert]]

The term "dest rect" is currently overloaded for nsImageFrame.

* Usage #1:
nsLayoutUtils::Draw*Image methods take a nsRect parameter "aDest".  This basically corresponds to the rect where a single copy of the image should be rendered to. In bug 624647, I'll be adding a new nsLayoutUtils method called "ComputeObjectDestRect" which will be about the same rect.

* Usage #2:
There's a function called "nsDisplayImage::GetDestRect()" which returns a gfxRect. It's only used inside of nsImageFrame. This appears to correspond to the nsDisplayImage's bounds, converted to dev pixels.

It's confusing to have these two different usages of the same term. Usage #2 has a more limited scope, so that one is easier to rename. Possible new names: GetDisplayRect, or GetBoundRect, or GetDestGFXRect.

Comment 1

[Daniel Holbert [:dholbert]]

(The gfxRect-flavored version was added in http://hg.mozilla.org/mozilla-central/rev/ed9e42293a6f#l7.12 for bug 650988. Adding dependency on that bug.)

Comment 2

[Seth Fowler [:seth] [:s2h]]

In a perfect world both of these might reasonably be considered "dest rects", but with different types. They do have different types, of course, but they're not really "semantic" types. It'd be nice if we had, say, Rect<Device> and Rect<CSS>, where things like Device and CSS are phantom types to denote the units involved.

Comment 3

[Seth Fowler [:seth] [:s2h]]

So it wasn't initially obvious to me, but I now think that nothing actually wants the current behavior of GetDestRect(). So I filed bug 1150704 about moving the algorithm from GetOpaqueRect() into GetDestRect(). That has the side effect of removing this ambiguity.

I'll go ahead and resolve this bug since we don't need to rename anything with this new plan.