Closed Bug 573953 Opened 13 years ago Closed 13 years ago

Zoom factors in reftest-zoom reftests have to be set carefully


(Core :: Graphics, defect)

Not set





(Reporter: roc, Assigned: roc)




(1 file)

Attached patch fixSplinter Review
Logic in reftest.js assumes that the reftest document's width in CSS pixels is (almost) exactly 800/zoom. However, what actually happens is that we set appunits per device pixel to floor(60/zoom) (in nsThebesDeviceContext::UpdateScaledAppUnits) and then the document's width in CSS pixels is 800*floor(60/zoom)/60, which can be significantly different from 800/zoom. Similar for height.

To deal with this, we should set the zoom factors used in reftests so 60/zoom is an integer.

Even then, due to numerical error, in nsThebesDeviceContext::UpdateScaledAppUnits we can find that float(mAppUnitsPerDevNotScaledPixel) / mPixelScale is just slightly less than an integer. So we should round it instead of truncating.
Attachment #453343 - Flags: review?(dbaron)
Couldn't we fix the logic in reftest.js to assume the document might be a little bigger?

This wouldn't hide any bugs, though, since what's happening in the code is that the zoom is being turned into a number such that 60/zoom is an integer, so we're not actually testing anything different, right?
Comment on attachment 453343 [details] [diff] [review]

I'm ok with this, but I think we should at least have a bug on file on not having to do this.
Attachment #453343 - Flags: review?(dbaron) → review+
Filed bug 576152.

The problem is that the document's CSS pixel width is smaller than expected by reftest.js, so we try to paint more pixels than there are.

The correct fix for reftest.js is probably to figure out what the document's actual CSS pixel width is, and just draw those pixels.
(and clear the rest of the canvas, or something)
Whiteboard: [needs landing]
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.