Last Comment Bug 468358 - <canvas> createPattern with repeat-x or repeat-y doesn't work correctly when used with fill()
: <canvas> createPattern with repeat-x or repeat-y doesn't work correctly when ...
Status: NEW
: html5
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
: 770134 (view as bug list)
Depends on:
Blocks: 622842
  Show dependency treegraph
 
Reported: 2008-12-07 12:30 PST by nospaming
Modified: 2015-10-13 16:56 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase from reporter (371 bytes, text/html)
2010-07-20 04:32 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details

Description nospaming 2008-12-07 12:30:49 PST
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 (⌚ UTC+1/+2) 2010-07-20 04:32:47 PDT
Created attachment 458624 [details]
Testcase from reporter
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2010-07-20 09:31:10 PDT
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] 2010-07-20 11:22:35 PDT
Correct; cairo has no notion of repeat-x/repeat-y, and it's pretty difficult to emulate it.
Comment 5 Stefan Kienzle 2013-02-04 23:03:22 PST
Any news?
Comment 6 Vladimir Vukicevic [:vlad] [:vladv] 2013-02-05 00:02:04 PST
Seems to work fine now, on Windows at least?  I'm guessing Azure may have fixed this...
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2013-02-05 01:19:56 PST
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 (⌚ UTC+1/+2) 2013-02-05 06:03:48 PST
WFM on Linux too.
Comment 9 Stefan Kienzle 2013-02-06 05:51:16 PST
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 [:bz] (still a bit busy) 2013-02-06 06:08:00 PST
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 11 :Ms2ger (⌚ UTC+1/+2) 2013-09-08 12:20:20 PDT
*** Bug 770134 has been marked as a duplicate of this bug. ***
Comment 12 martin 2014-01-01 14:04:49 PST
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] 2014-06-03 10:38:14 PDT
(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 [:bz] (still a bit busy) 2014-06-03 12:07:33 PDT
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] 2014-06-03 14:04:45 PDT
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!

Note You need to log in before you can comment on or make changes to this bug.