Closed
Bug 612033
Opened 14 years ago
Closed 5 years ago
Calling context.setTransform( 0, 0, 0, 0, 0, 0 ) breaks canvas even after calling setTransform with non-zero values.
Categories
(Core :: Graphics: Canvas2D, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: bugzilla, Unassigned)
References
Details
(Keywords: testcase)
Attachments
(2 files, 1 obsolete file)
|
(deleted),
text/html
|
Details | |
|
(deleted),
patch
|
joe
:
review-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/534.7 (KHTML, like Gecko) Chrome/7.0.517.41 Safari/534.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12 ( .NET CLR 3.5.30729; .NET4.0C)
After calling context.setTransform( 0, 0, 0, 0, 0, 0 ) on a canvas element's 2d context, subsequent calls to setTransform do not set the context's transformation matrix correctly. Drawing functions do not show expected results, even after resetting the transformation matrix to the identity via context.setTransform( 1, 0, 0, 1, 0, 0 ).
Reproducible: Always
Steps to Reproduce:
1. Call context.setTransform( 0, 0, 0, 0, 0, 0 );
2. Call context.setTransform( 1, 0, 0, 1, 0, 0 ); resetting to identity matrix should cause future draws to show expected image results
3. Call any context drawing function [ for the example case, I used ctx.fillRect( 10, 10, 80, 80 ); ]
4. Compare to another canvas who has not had setTransform( 0, 0, 0, 0, 0, 0 ) called
Actual Results:
Canvas without the zero-transformation matrix shows a black square at (10,10) with width and height = 80; canvas with zero-transformation and subsequent reset to identity shows nothing.
Expected Results:
Both canvasses should have shown the same black square.
Test script:
<html>
<head>
<title>Firefox Zero Transformation Matrix Bug</title>
<script type="text/javascript">
window.onload = function()
{
// get the contexts
var cnv1 = document.getElementById( 'test1' ),
ctx1 = cnv1.getContext('2d'),
cnv2 = document.getElementById( 'test2' ),
ctx2 = cnv2.getContext('2d');
// draw a rect to the first context
// set the transform to identity (to show that ctx2's call to the same function does not have the same result)
ctx1.setTransform( 1, 0, 0, 1, 0, 0 );
ctx1.fillRect( 10, 10, 80, 80 );
// here's where the problem is
ctx2.setTransform( 0, 0, 0, 0, 0, 0 );
// set to identity
ctx2.setTransform( 1, 0, 0, 1, 0, 0 );
ctx2.fillRect( 10, 10, 80, 80 );
}
</script>
<style type="text/css">
canvas{ border: 1px solid black; }
</style>
</head>
<body>
<canvas id="test1" width="100" height="100"></canvas>
<canvas id="test2" width="100" height="100"></canvas>
</body>
</html>
This attachment shows the test case where one context has had a zero-transformation matrix set via setTransform against another canvas. The first canvas has not had the setTransform called that breaks the canvas.
|
||
Comment 2•13 years ago
|
||
The problem is that trying to set a singular matrix as the transform causes cairo to error out and refuse to do anymore paint operations. This patch makes transform() and setTransform() error out on singular matrices. (I choose this behavior since the spec is silent on it.) It also makes setTransform() consistent with transform() by silently returning on args of Infinity or Nan.
Assignee: nobody → benjamin
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #533664 -
Flags: review?(joe)
|
||
Comment 4•13 years ago
|
||
Comment on attachment 533664 [details] [diff] [review]
raise an error on singular matrices
Review of attachment 533664 [details] [diff] [review]:
-----------------------------------------------------------------
I like this. Because it's technically an interface change, though, I'd like roc to sr.
::: content/canvas/src/nsCanvasRenderingContext2D.cpp
@@ +1427,5 @@
> NS_IMETHODIMP
> nsCanvasRenderingContext2D::SetTransform(float m11, float m12, float m21, float m22, float dx, float dy)
> {
> if (!FloatValidate(m11,m12,m21,m22,dx,dy))
> + return NS_OK;
Can you make SetTransform behave the same as Transform in a separate patch? It can be committed separately.
::: content/canvas/test/test_canvas.html
@@ +21342,5 @@
> + try {
> + ctx.setTransform(0, 0, 0, 0, 0, 0);
> + ok(false, "setTransform didn't throw");
> + } catch (e if e instanceof DOMException) {
> + ok(e.code == DOMException.SYNTAX_ERR);
No "wrong exception"?
Attachment #533664 -
Flags: superreview?(roc)
Attachment #533664 -
Flags: review?(joe)
Attachment #533664 -
Flags: review+
Shouldn't we just handle singular transforms correctly instead of throwing? That means while the transform is singular, drawing operations shouldn't paint anything (except for effects due to non-over operators). That should fix bug 659201 too.
|
|
||
Comment 6•13 years ago
|
||
(In reply to comment #5)
> Shouldn't we just handle singular transforms correctly instead of throwing?
> That means while the transform is singular, drawing operations shouldn't
> paint anything (except for effects due to non-over operators). That should
> fix bug 659201 too.
Is that a useful behavior to have? I see this is what WebKit does, but I'm not sure where one would use it.
|
||
Comment 7•13 years ago
|
||
(In reply to comment #5)
> Shouldn't we just handle singular transforms correctly instead of throwing?
> That means while the transform is singular, drawing operations shouldn't
> paint anything (except for effects due to non-over operators). That should
> fix bug 659201 too.
I would agree with that, it is quite standard to not generate exceptions or otherwise halt execution on floating point error-cases (like divide by zero). For example this is why by default FPU exceptions don't raise signals.
(In reply to comment #6)
> Is that a useful behavior to have? I see this is what WebKit does, but I'm
> not sure where one would use it.
Imagine you're animating the transform and it happens to pass through a singular value. You don't want to suddenly have your script get an exception. You'd end up having to test for a singular matrix yourself, which would suck.
Also, even when there isn't a sane behavior we tend to not throw exceptions because it's the "bottom of the slippery slope" state: if some browsers throw exceptions and others don't, the ones that don't seem to work better for users. But in this case there is a perfectly sane behavior, so we should just use it.
|
|
||
Comment 9•13 years ago
|
||
Here is a simple patch with a hopefully comprehensive test.
Attachment #533664 -
Attachment is obsolete: true
Attachment #545314 -
Flags: review?(joe)
Attachment #533664 -
Flags: superreview?(roc)
|
|
||
Comment 10•13 years ago
|
||
Comment on attachment 545314 [details] [diff] [review]
ignore all drawing operations when a singular transform is in effect
I really don't like having to sprinkle if (singular) all over the place. Can we not fix Cairo to allow us to get a cairo_t out of permanent error state after setting a singular matrix?
Attachment #545314 -
Flags: review?(joe) → review-
|
|
||
Comment 11•13 years ago
|
||
I'm not sure(In reply to comment #10)
> Comment on attachment 545314 [details] [diff] [review] [review]
> ignore all drawing operations when a singular transform is in effect
>
> I really don't like having to sprinkle if (singular) all over the place. Can
> we not fix Cairo to allow us to get a cairo_t out of permanent error state
> after setting a singular matrix?
I'm not sure. Can't cairo being in an error state cause the whole context to be put in an invalid state such that simply changing the error status won't work?
FWIW, this is nearly exactly what WebKit does, too.
|
|
||
Comment 12•13 years ago
|
||
Moreover, suppose we could reset cairo's error state. Where would we do it? We can't do it after someone sets a singular matrix because then drawing operations would use a different transform. We can't reset the error state when the singular matrix is removed because some cairo operations like save/restore and setting styles must still be supported, and cairo refusing to do anything when it's in error.
|
|
||
Comment 13•13 years ago
|
||
Comment on attachment 545314 [details] [diff] [review]
ignore all drawing operations when a singular transform is in effect
Jeff, what do you think?
Attachment #545314 -
Flags: feedback?(jmuizelaar)
|
|
||
Comment 14•13 years ago
|
||
Jeff?
|
|
||
Comment 15•13 years ago
|
||
FWIW, the HTML5 spec now explicitly states how to handle singular transformations.
|
||
Updated•13 years ago
|
QA Contact: canvas.2d → jmuizelaar
|
|
||
Updated•12 years ago
|
Assignee: benjamin → nobody
Status: ASSIGNED → NEW
The example code works as expected on Nightly: https://codepen.io/SaschaNaz/pen/rXBMZo
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•