If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Status

()

Core
Canvas: 2D
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: vlad, Assigned: vlad)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This patch fixes Mac frame updating and image code to get the BGR and other
behaviour right; it also shuffles around some default values to fix bug 293221.
Created attachment 183771 [details] [diff] [review]
canvas-mac-fix.patch
Assignee: nobody → vladimir
Status: NEW → ASSIGNED
Attachment #183771 - Flags: review?(shaver)
Comment on attachment 183771 [details] [diff] [review]
canvas-mac-fix.patch

>@@ -548,22 +557,35 @@
>             return rv;
>         }
> 
>+        nsCOMPtr<nsIImage> img(do_GetInterface(mImageFrame));
>+        PRBool topToBottom = img->GetIsRowOrderTopToBottom();
>+#ifdef XP_MACOSX
>+        topToBottom = PR_FALSE;
>+#endif

Is there a bug on GetIsRowOrderTopToBottom not returning the 
right thing on OS X?  If not, can you file, and cite the bug
number here either way?

>+            PRUint8 *outrowrgb, *outrowalpha;
>+            if (topToBottom) {
>+                outrowrgb = rgbBits + (rgbStride * j);
>+                outrowalpha = alphaBits + (alphaStride * j);
>+            } else {
>+                outrowrgb = rgbBits + (rgbStride * (mHeight - j - 1));
>+                outrowalpha = alphaBits + (alphaStride * (mHeight - j - 1));
>+            }

I'd find that easier to read if you manually hoisted the j 
vs mHeight-j-1 value to a local, but I'm not quite sure what
you'd call it at that point.  rowOffset?

>@@ -909,6 +930,7 @@
> NS_IMETHODIMP
> nsCanvasRenderingContext2D::GetShadowColor(nsAString& color)
> {
>+    StyleColorToString(mColorStyles[STYLE_SHADOW], color);
>     return NS_OK;
> }

Unrelated change here?

>@@ -1074,7 +1096,9 @@
> {
>     cairo_line_cap_t cap;
> 
>-    if (capstyle.EqualsLiteral("round"))
>+    if (capstyle.EqualsLiteral("butt"))
>+        cap = CAIRO_LINE_CAP_BUTT;
>+    else if (capstyle.EqualsLiteral("round"))
>         cap = CAIRO_LINE_CAP_ROUND;
>     else if (capstyle.EqualsLiteral("square"))
>         cap = CAIRO_LINE_CAP_SQUARE;

And here?

>@@ -1279,11 +1305,11 @@
>     else CANVAS_OP_TO_CAIRO_OP("destination-in", IN_REVERSE)
>     else CANVAS_OP_TO_CAIRO_OP("destination-out", OUT_REVERSE)
>     else CANVAS_OP_TO_CAIRO_OP("destination-over", OVER_REVERSE)
>-    else CANVAS_OP_TO_CAIRO_OP("lighter", SATURATE)
>+    else CANVAS_OP_TO_CAIRO_OP("lighter", ADD)  // XXX

XXX what?

>     PRBool useBGR;
> 
>     if (format == gfxIFormats::RGB || format == gfxIFormats::BGR) {
>         useBGR = (format & 1);
> 
>+#ifdef IS_BIG_ENDIAN
>+        useBGR ^= 1;
>+#endif

Ugh.  useBGR = !useBGR instead?  I'm willing to beg.

I wish there wasn't so much endian-stuff all over here, but I don't see
a good alternative.

r-, but you were going to put up a patch with the right
GetIsRowOrderTopToBottom fix anyway, so no biggie!
Attachment #183771 - Flags: review?(shaver) → review-
Created attachment 183879 [details] [diff] [review]
patch #2


This patch fixes this bug, bug 293221 (default values for canvas ops), and bug
294572 (the image rendering bug).  Horray for gcc bugs and for confusingly
named cairo operators (which get fixed once we update to CVS cairo).
Attachment #183771 - Attachment is obsolete: true
Attachment #183879 - Flags: review?(shaver)
Comment on attachment 183879 [details] [diff] [review]
patch #2

Yay, much better. r=shaver.
Attachment #183879 - Flags: review?(shaver)
Attachment #183879 - Flags: review+
Attachment #183879 - Flags: approval1.8b2+
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.