Taming coordinate systems

Posted by: stak
Tags: mozilla
Posted on: 2013-05-30 12:31:31

For a while now we've had problems stemming from having to deal with too many coordinate systems. I blogged about this before and the problem has only gotten worse. The AsyncPanZoomController class in particular deals with a variety of coordinate systems and it's often not clear which coordinate system a particular variable is in.

To try and deal with these code complexity issues, Anthony Jones suggested adding template parameters to some of the gfx classes so that we can enforce unit conversions at compile-time and annotate variables with which units they're in. I filed bug 865735 for this, and after some discussion with roc, Bas, jrmuizel, BenWa, tn, Cwiiis and some others, we agreed on a way to do this. I landed that patch and it was merged to m-c today.

The patch allows for incrementally adding units to uses of gfx::Point, gfx::Size, and gfx::Rect (and their Int variations). By default all instances of these classes are tagged as UnknownUnits. As we update bits of code they will be changed to things like CSSPixels, LayerPixels, ScreenPixels, and so on, depending on what they are. I've been working on a couple of patches that start adding this extra information to pieces of code; these patches can be found on bug 877726 and bug 877728. Doing this is slow work, but very parallelizable, so I would appreciate any help I can get. If you know of some graphics code that uses these classes, and you know what units they data they hold are in, please file a bug with patches to convert them. Feel free to CC me and/or make it depend on bug 865735.

They key files that define the templates and units are located at gfx/2d/Point.h, gfx/2d/Rect.h, and layout/base/Units.h. gfx::Point is now just a typedef to gfx::PointTyped<UnknownUnits>. Replacing UnknownUnits with CSSPixel gives us the new type gfx::PointTyped<CSSPixel> to represent points in CSS pixel coordinate systems. Since this is long and unwieldy to type, we have typedef'd this to CSSPoint. Similar changes will be done for the other classes (e.g. CSSIntPoint, CSSRect) and units (e.g. LayerPoint) as we start propagating them. The neat thing about the templating structure we came up with is that gfx::PointTyped<CSSPixel> actually extends from CSSPixel, so we can add methods (e.g. conversion to app units) there that are available on all CSSPoint instances but not other unit classes.

As far as I understand things, layout code currently always keeps nsPoint instances in app units (1/60 of a CSS pixel). nsIntPoint, however, can be used for a variety of units, and many of these (particularly the ones outside layout/) should be converted to gfx::IntPointTyped<something>. What layout code refers to as "device pixels" is generally not the same thing that graphics code refers to as "device pixels", so I'm trying to avoid using the term "device pixels" entirely in graphics code - I prefer to use "layer pixels", "display pixels", and "screen pixels" as needed. For non-OMTC platforms "screen pixels" and "layer pixels" should be equivalent, and for platforms without hidpi adjustments, "display pixels" and "screen pixels" should be the same.

While converting code I've run into many places where new point variables are constructed using the .x and .y members of a different point variable. It's important to ensure that such code is carefully audited to make sure that the old and new variables are in the same units. If it's not clear, it's best to stick in a call to FromUnknownUnits(...) so that it is implicitly flagged for follow-up when other nearby code gets converted.

[ Add a new comment ]