SVG objects with patterned fills in files saved by some versions of Inkscape show incorrect patterns in Firefox 3.0.1

RESOLVED FIXED

Status

()

Core
SVG
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Will Pittenger, Assigned: longsonr)

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(URL)

Attachments

(8 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: 

Examine the fill for the checkered pattern in the start/finish line of the page given.  It should be a checker board with white and black squares alternating.  For me, it doesn't show that.  Instead, I see the attached images.  The first shows what I see.  The second shows what should be visible.  The third is the problem SVG with everything except that object deleted.

Reproducible: Always

Steps to Reproduce:
1. View the SVG
Actual Results:  
Pattern won't be displayed correctly

Expected Results:  
Checker board pattern to display as alternating white and black squares
(Reporter)

Comment 1

10 years ago
Created attachment 332098 [details]
This shows what I see when I use FF 3.0.1 to view the SVG.

This shows what I see when I use FF 3.0.1 to view the SVG.
(Reporter)

Comment 2

10 years ago
Created attachment 332099 [details]
What the pattern should look like
(Reporter)

Comment 3

10 years ago
Created attachment 332100 [details]
Simplified SVG showing just the problem object

Comment 4

9 years ago
Created attachment 399678 [details]
Testcase 1 (broken)

Even more reduced testcase.

Referencing a transformed pattern from a transformed element via |xlink:href| causes the problem.

<rect/> -> <pattern
             @xlink:href -> <pattern/>
           />

Comment 5

9 years ago
Created attachment 399679 [details]
Testcase 2 (works)

Even more reduced testcase.

Referencing a transformed pattern from a transformed element directly doesn't causes the problem.

<rect/> -> <pattern/>

Updated

9 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Version: unspecified → Trunk

Comment 6

9 years ago
Created attachment 399680 [details]
Testcase 1 + 2 (broken)

Mysteriously, rendering pattern referenced directly is failed when |xlink:href|ed pattern was rendered before.


<rect/> -> <pattern
             @xlink:href -> <pattern/>
           />
<rect/> -> <pattern/>

both are wrong rendering.

Updated

9 years ago
Attachment #399678 - Attachment description: Testcase (broken) → Testcase 1 (broken)

Updated

9 years ago
Attachment #399679 - Attachment description: Testcase (works) → Testcase 2 (works)

Comment 7

9 years ago
Created attachment 399681 [details]
Testcase 2 + 1 (works)

Rendering |xlink:href|ed pattern is when pattern referenced directly was rendered before.


<rect/> -> <pattern/>
<rect/> -> <pattern
             @xlink:href -> <pattern/>
           />

both are O.K.
(Assignee)

Comment 8

9 years ago
Created attachment 408268 [details] [diff] [review]
patch
Assignee: nobody → longsonr
Attachment #408268 - Flags: review?(roc)
(Assignee)

Comment 9

9 years ago
Thanks for the testcases Takeshi. I would not have been able to fix this without them.
   if (!mPaintLoopFlag) {
     mPaintLoopFlag = PR_TRUE;

Should we be setting/checking mPaintLoopFlag on patternFrame?

Should we just be calling nsSVGPatternFrame::PaintPattern on patternFrame instead?
(Assignee)

Comment 11

9 years ago
(In reply to comment #10)
>    if (!mPaintLoopFlag) {
>      mPaintLoopFlag = PR_TRUE;
> 
> Should we be setting/checking mPaintLoopFlag on patternFrame?

I can change this as it's the only member variable that paintPattern sets and I guess it does make more sense to set it on the pattern you're rendering. However, I don't think it makes much difference. If there is a loop then either you come back round to the current frame or the linked frame and you'll see the flag set at some point either on the current frame or the linked frame. 

> 
> Should we just be calling nsSVGPatternFrame::PaintPattern on patternFrame
> instead?

This we can't do. We need to start from the current frame not the linked frame to get pattern attributes. e.g. with pattern x="3" xlink:href="..." we need to get x="3"
(Assignee)

Comment 12

9 years ago
Created attachment 408293 [details] [diff] [review]
patternFrame->mPaintLoop
Attachment #408268 - Attachment is obsolete: true
Attachment #408293 - Flags: review?(roc)
Attachment #408268 - Flags: review?(roc)
(Assignee)

Comment 13

9 years ago
pushed http://hg.mozilla.org/mozilla-central/rev/4d40ebb002a3
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Comment 14

9 years ago
Comment on attachment 408293 [details] [diff] [review]
patternFrame->mPaintLoop

I think it might be worth taking this in 1.9.2 although I don't know how prevalent this is in Inkscape generated files. Pretty simple low risk fix that includes a test.
Attachment #408293 - Flags: approval1.9.2?
Attachment #408293 - Flags: approval1.9.2? → approval1.9.2+
(Assignee)

Comment 15

9 years ago
pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/94bda78c3ab9
status1.9.2: --- → beta1-fixed
You need to log in before you can comment on or make changes to this bug.