Closed Bug 547924 Opened 15 years ago Closed 15 years ago

Firefox composites canvas tags into the page incorrectly.

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: mozilla2, Assigned: roc)

References

Details

Attachments

(3 files)

Reporter's testcase
1.58 KB, text/html
Details
partial fix
799 bytes, patch
vlad
: review-
Details | Diff | Splinter Review
fix
1.40 KB, patch
vlad
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)

Firefox composites canvas tags blending the edges with 0,0,0,0 if the canvas is displayed larger than it's internal size. This causes undesirable artifacts and inconsistencies with the img tag.

See sample below

Note: The larger the difference in size of the 

Reproducible: Always

Steps to Reproduce:
1. Run the code below

Actual Results:  
Canvas tags have incorrectly rendered border pixels.

Expected Results:  
Canvas should not blend their edge pixels with 0,0,0,0 making them inconsistent with img tags and other tags.

<!DOCTYPE html>
<html>
<head>
<title>Canvas Compositing Issue</title>
</head>
<body>
<pre>
Below is a 50x50 pixel canvas and a 5x5 pixel canvas.
but using CSS to display them at 100x100 pixels.
Each has a 10pixel black CSS border.

We clear the canvases to solid black, then draw a red triangle.
We then copy that canvas to corresponding img tags using toDataURL.

The canvases should appear identical to the img tags but they don't.
This is because Firefox is compositing canvas tags into the scene
differently than img tags.<hr/>
</pre>
<div>
2d canvas<br/>
<canvas id="canvas1" width="50" height="50" style="width: 100px; height: 100px; border: 10px solid black;"></canvas>
<canvas id="canvas2" width="5" height="5" style="width: 100px; height: 100px; border: 10px solid black;"></canvas>
</div>
<hr/>
img tag<br/>
<img id="image1" style="width: 100px; height: 100px; border: 10px solid black;"/>
<img id="image2" style="width: 100px; height: 100px; border: 10px solid black;"/>
<script>
for (var ii = 0; ii < 2; ++ii) {
  var img = document.getElementById('image' + (ii + 1));
  var canvas = document.getElementById('canvas' + (ii + 1));
  var ctx = canvas.getContext("2d");
  // fill canvas with black
  ctx.fillStyle = "rgba(0, 0, 0, 255)"
  ctx.fillRect(0, 0, canvas.width, canvas.height);
  // draw red rectangle on top
  ctx.fillStyle = "rgba(255, 0, 0, 255)"
  ctx.beginPath();
  var q = canvas.width / 4;
  ctx.moveTo(q * 2, q);
  ctx.lineTo(q, q * 3);
  ctx.lineTo(q * 3, q * 3);
  ctx.lineTo(q * 2, q);
  ctx.fill();
  // copy result to img tag
  img.src = canvas.toDataURL();
}
</script>
</body>
</html>
Please use the Add an attachment button to add your testcase.
Attached file Reporter's testcase — 
The <canvas> and <img> look the same to me over here on Mac.  Reporter, mind attaching a screenshot of what the testcase looks like for you?
Or is the issue the thin grayish line near the top of the canvases?
Yes, on the Mac the bug is the single gray light at the top. On Windows it's much worse.
Yes, on the Mac the bug is the single gray light at the top. On Windows and Linux it's much worse.
roc, do we have existing bugs on this?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Not that I know of.

The problem is that we're compositing with EXTEND_NONE. We should be using EXTEND_PAD. That doesn't fix it on Mac though, because we don't support EXTEND_PAD with Quartz. I'm not quite sure why the image works OK but the canvas doesn't.

I'm not sure what to do here other than support EXTEND_PAD properly on Quartz.
Attached patch partial fix — — Splinter Review 
This does the right thing at the layout level. It should fix the bug for Windows, but it doesn't for Mac because cairo doesn't support EXTEND_PAD with Quartz in general.
Assignee: nobody → roc
Attachment #428634 - Flags: review?(vladimir)
Whiteboard: [needs review]
Assignee: roc → nobody
Assignee: nobody → roc
Comment on attachment 428634 [details] [diff] [review]
partial fix

I don't know what the current state of PAD is -- does this hurt perf at all, esp. in the common no-scale case?
I don't know right now either, but I can look.
Comment on attachment 428634 [details] [diff] [review]
partial fix

This needs to go into gfx/layers/*/{BasicLayers.cpp,CanvasLayerOGL.cpp} now, though the question about the PAD situation still stands...
Attachment #428634 - Flags: review?(vladimir) → review-
Attached patch fix — — Splinter Review 
Updated BasicLayers to also use EXTEND_PAD.

I reckon we should just go with EXTEND_PAD on X and presume that cairo does the right thing. Where there are still broken drivers, cairo should just map it to EXTEND_NONE internally.
Attachment #446085 - Flags: review?(vladimir)
Comment on attachment 446085 [details] [diff] [review]
fix

Sure, let's see what happens.
Attachment #446085 - Flags: review?(vladimir) → review+
Whiteboard: [needs review] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/e328183c0602

This fixed layout/reftests/bugs/488692-1.html on Linux/Windows, so I marked that test as passing.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Depends on: 594319
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: