Closed
Bug 573953
Opened 14 years ago
Closed 14 years ago
Zoom factors in reftest-zoom reftests have to be set carefully
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: roc)
References
Details
Attachments
(1 file)
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter 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)
Comment 1•14 years ago
|
||
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 2•14 years ago
|
||
Comment on attachment 453343 [details] [diff] [review]
fix
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+
Assignee | ||
Comment 3•14 years ago
|
||
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.
Assignee | ||
Comment 4•14 years ago
|
||
(and clear the rest of the canvas, or something)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs landing]
Assignee | ||
Comment 5•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in
before you can comment on or make changes to this bug.
Description
•