430827 - Add defensive measures against higher-level code clobbering the CTM

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[Zack Weinberg (:zwol)]

It is currently possible for any code that has access to a gfxContext to call SetMatrix() and wipe out the current coordinate transformation.  We want to use the CTM to go from layout to device units (bug 430825), so as a defensive measure we should make the compiler enforce a usage discipline of the CTM where it's impossible to clobber it, only push and pop additional transformations.

Robert suggests a helper class gfxContextAutoSaveMatrix.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

No, we should not do this.  Code should know what it's doing, and we shouldn't be hiding functionality -- Thebes isn't only for layout's usage.

Comment 2

[Zack Weinberg (:zwol)]

I'm leaving this closed for the moment because apparently there are objections to using the CTM for layout-to-device at all, so it may be moot.  

However, I disagree with your characterization of this as "hiding functionality".  By itself, a helper class that lets you manipulate the CTM in a stack-like fashion (with exception safety) is additional, useful functionality.  Anyone who comes to this interface with PostScript or PDF in mind will naturally want to do things this way.  It also makes a hierarchial rendering process where higher nodes wish to control the location of lower nodes very easy to implement.

Not providing any other mode of operation - that is, not allowing use of the existing SetMatrix() and IdentityMatrix() methods from outside some set of privileged code - is more debatable, but I would argue that there is a broad class of bugs that it renders impossible, and that that is rather more valuable than the rare code that really does need to clobber the matrix.  Note that PDF gets along just fine with no matrix-clobber operator at all.

As a middle way, I would be fine with a stack helper whose normal usage involved augmenting the CTM, but had a special method that did clobber it, for use only when there is no alternative.  (I think situations where the matrix gets modified and then not reset are a much bigger concern than situations where it gets modified to something weird.)

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #0)
> we should make the compiler enforce a usage discipline of the CTM where
> it's impossible to clobber it, only push and pop additional transformations.

That's the part that's WONTFIX.

> Robert suggests a helper class gfxContextAutoSaveMatrix.

That's fine; there's an existing gfxContextAutoSaveRestore that can be used to do this already, though a gfxContextAutoMultiplyMatrix(const gfxMatrix& m) sounds like a decent addition.  Save/Restore is cheap though, so I'm not sure if it should try to do anything fancy beyond Save(); Multiply(m); ... Restore();.