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.

Tracking

()

Status:

RESOLVED WORKSFORME

People

(Reporter: bugzilla, Unassigned)

References

Details

(Keywords: testcase)

Attachments

(2 files, 1 obsolete file)

working test case 14 years ago Scott 1003 bytes, text/html		Details
raise an error on singular matrices 13 years ago :Benjamin Peterson 4.21 KB, patch	joe : review+	Details \| Diff \| Splinter Review
ignore all drawing operations when a singular transform is in effect 13 years ago :Benjamin Peterson 14.64 KB, patch	joe : review- joe : feedback? jrmuizel	Details \| Diff \| Splinter Review

Scott

Reporter

Description

•

14 years ago

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>

Scott

Reporter

Comment 1

•

14 years ago

Attached file working test case — Details

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.

(mostly gone) XtC4UaLL [:xtc4uall]

Updated

•

14 years ago

Keywords: testcase

:Benjamin Peterson

Comment 2

•

13 years ago

Attached patch raise an error on singular matrices (obsolete) — Details — Splinter Review

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)

Joe Drew (not getting mail)

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+

Robert O'Callahan (:roc) (email my personal email if necessary)

Comment 5

•

13 years ago

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.

:Benjamin Peterson

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.

Benoit Jacob [:bjacob] (mostly away)

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.

Robert O'Callahan (:roc) (email my personal email if necessary)

Comment 8

•

13 years ago

(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.

:Benjamin Peterson

Comment 9

•

13 years ago

Attached patch ignore all drawing operations when a singular transform is in effect — Details — Splinter Review

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)

Joe Drew (not getting mail)

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-

:Benjamin Peterson

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.

:Benjamin Peterson

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.

Joe Drew (not getting mail)

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)

:Benjamin Peterson

Comment 14

•

13 years ago

Jeff?

:Benjamin Peterson

Comment 15

•

13 years ago

FWIW, the HTML5 spec now explicitly states how to handle singular transformations.

Ali Juma [:ajuma]

Updated

•

12 years ago

QA Contact: canvas.2d → jmuizelaar

:Benjamin Peterson

Updated

•

12 years ago

Assignee: benjamin → nobody

Status: ASSIGNED → NEW

Kagami Rosylight [:saschanaz] (they/them)

Comment 16

•

5 years ago

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.