468358 - <canvas> createPattern only works correctly with repeat (and not with repeat-x, repeat-y or no-repeat) when used with fill()

Status: REOPENED

(Core :: Graphics: Canvas2D, defect, P3)

Description

[nospaming]

User-Agent:       Opera/9.62 (Windows NT 5.1; U; en) Presto/2.1.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1

If you create a pattern with repeat-x, set it as fillStyle and fill a rectangular area, everything works fine. The image only gets repeated horizontally.
But if you use repeat-y, it works the same as repeat.

Reproducible: Always

Steps to Reproduce:
// context is a 2d-context
var img = new Image();
img.src = 'http://www.mozilla.com/img/tignish/about/logo/download/logo-only-preview.png';

// after loading of the image
context.fillStyle = context.createPattern(img, 'repeat-y');
context.fillRect(0, 0, 200, 200);
Actual Results:  
It behaves the same as if the pattern had been created with
context.createPattern(img, 'repeat');

Expected Results:  
Repeat vertically only:
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-createpattern

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Testcase from reporter

Comment 2

[Boris Zbarsky [:bzbarsky]]

Neither repeat-x nor repeat-y work; I'm not sure what comment 0 was talking about.  The relevant code:

1736     if (repeat.IsEmpty() || repeat.EqualsLiteral("repeat")) {
1737         extend = gfxPattern::EXTEND_REPEAT;
1738     } else if (repeat.EqualsLiteral("repeat-x")) {
1739         // XX
1740         extend = gfxPattern::EXTEND_REPEAT;
1741     } else if (repeat.EqualsLiteral("repeat-y")) {
1742         // XX
1743         extend = gfxPattern::EXTEND_REPEAT;
1744     }

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Correct; cairo has no notion of repeat-x/repeat-y, and it's pretty difficult to emulate it.

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Related failures:

http://www.w3c-test.org/html/tests/approved/canvas/2d.pattern.paint.repeatx.coord1.html
http://www.w3c-test.org/html/tests/approved/canvas/2d.pattern.paint.repeatx.outside.html
http://www.w3c-test.org/html/tests/approved/canvas/2d.pattern.paint.repeaty.coord1.html
http://www.w3c-test.org/html/tests/approved/canvas/2d.pattern.paint.repeaty.outside.html

Comment 5

[Stefan Kienzle]

Any news?

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Seems to work fine now, on Windows at least?  I'm guessing Azure may have fixed this...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Certainly the code I cited in comment 2 now looks like this:

1388   if (repeat.IsEmpty() || repeat.EqualsLiteral("repeat")) {
1389     repeatMode = CanvasPattern::REPEAT;
1390   } else if (repeat.EqualsLiteral("repeat-x")) {
1391     repeatMode = CanvasPattern::REPEATX;
1392   } else if (repeat.EqualsLiteral("repeat-y")) {
1393     repeatMode = CanvasPattern::REPEATY;
1394   } else if (repeat.EqualsLiteral("no-repeat")) {
1395     repeatMode = CanvasPattern::NOREPEAT;
1396   }

and the testcase in comment 0 worksforme (on Mac).

Stefan, are you still able to reproduce this issue in a nightly, since you commented?

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

WFM on Linux too.

Comment 9

[Stefan Kienzle]

Sorry, you are right. This example works on my Windows 7 (64bit) on Firefox (I've tested v17-21).

But if you look at this example it doesn't work:
http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_canvas_createpattern

I think context.fill() does'n work correctly.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Ah, yes.  For fillRect() we adjust the rect to take the repeat value into account, but for fill() we can't do that. And the code at http://hg.mozilla.org/mozilla-central/file/bc108d2ce8d1/content/canvas/src/CanvasRenderingContext2D.cpp#l242 sets ExtendMode to "REPEAT" if the canvas pattern's repeat mode is not NOREPEAT.

Comment 12

[martin]

note also that in the example at http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_canvas_createpattern

the 'no-repeat' option doesn't work correctly either: there is something extending to the right of and below the image, and a coloured rectangle in the rest of the space

Comment 13

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Boris Zbarsky [:bz] from comment #10)
> Ah, yes.  For fillRect() we adjust the rect to take the repeat value into
> account, but for fill() we can't do that.
> ...

Boris, "Can't" above is because it's against the standard or some issue in our code or we just didn't implement it, or...?

Comment 14

[Boris Zbarsky [:bzbarsky]]

What we do for fillRect() is just change the rect to fill to take into account the repeat value and then fill that.

But for fill(), there is no rect to change.  There's just a path.  We _could_ try doing something like taking the rect that would actually be covered by the pattern and building some new path from that and the path we're filling, but it's not obvious to me how this would work, esp for self-intersecting paths.  No standard issue; just pure implementation bits.

Comment 15

[Milan Sreckovic [:milan] (needinfo for best results)]

Thanks, that clears it up.  Example like in comment 9:
...
var pat=ctx.createPattern(img,direction);
ctx.rect(0,0,150,100);
ctx.fillStyle=pat;
ctx.fill();
...
is something where we always fill in both directions, while webkit does differentiate between repeat-x/repeat-y and repeat.  Anyway, thanks for the info!

Comment 16

[andreabogazzi79]

So this is a wontfix?

http://jsfiddle.net/emeagcnx/22/

with fill and fillText the pattern works just on the 'repeat' mode while 'no-repeat', 'repeat-x', 'repeat-y' in addition with a translate, fails.

Safari can do it good.

Comment 17

[Milan Sreckovic [:milan] (needinfo for best results)]

More of a "we'll take a patch".  The example above works with fillRect(), but not with fill().

Comment 18

[Milan Sreckovic [:milan] (needinfo for best results)]

Btw, no-repeat doesn't work either.  Use jsfiddle from comment 16 - the original test case, with fillRect() does work.

Comment 19

[Milan Sreckovic [:milan] (needinfo for best results)]

And once no-repeat works, we may just need to do the small edit in the code mentioned in comment 10.

(And, yes, I realized I repeated in comment 18 what was already mentioned in comment 12 :)

Comment 20

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to martin from comment #12)
> note also that in the example at
> http://www.w3schools.com/tags/tryit.
> asp?filename=tryhtml5_canvas_createpattern
> 
> the 'no-repeat' option doesn't work correctly either: there is something
> extending to the right of and below the image, and a coloured rectangle in
> the rest of the space

This is basically us doing "clamp" instead of "repeat" (good), but not doing any kind of a clipping (bad)

Comment 22

[andreabogazzi79]

This is still behaving uncorrectly as today.
Is there a complexity wall or just the library powering firefox canvas can't do this kind of clipping?

Comment 23

[Mayank Bansal]

Direct links do not work, and we diligently track w3c test-suite now.

Comment 24

[Mayank Bansal]

Claas pointed out that this bug is infact still valid and the testcase at http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_canvas_createpattern doesnt work as expected in Firefox.
Reopening the bug.