Status

()

Core
SVG
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: longsonr, Assigned: longsonr)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Implement feTile filter
(Assignee)

Comment 1

11 years ago
Created attachment 258411 [details] [diff] [review]
implement tile filter
Attachment #258411 - Flags: review?(tor)

Comment 2

11 years ago
This patch will need carioification given the recent changes to filters.

Would "GetSourceRegion" be a more self-documenting name than "GetReferencedRegion"?

My reading of the SVG specification is that the union (ie, the old behavior of the region tracking) is what should be tiled, not the whole area.  If that is the case, your changes to add a subregion type seem unnecessary.

You are copying individual pixels - it seems with a bit more fun modular math you could copy entire rows.

Updated

11 years ago
Blocks: 311029
(Assignee)

Comment 3

11 years ago
(In reply to comment #2)
> My reading of the SVG specification is that the union (ie, the old behavior of
> the region tracking) is what should be tiled, not the whole area.  If that is
> the case, your changes to add a subregion type seem unnecessary.

I'm going by http://www.w3.org/TR/SVG/filters.html#FilterPrimitiveSubRegion where it talks about feTile using the entire region and not the previous filter.

This is borne out by the test case 
http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-tile-01-b.html
in which the tile has no attributes so it would not tile at all if it did not use the whole area as a source.

> 
> You are copying individual pixels - it seems with a bit more fun modular math
> you could copy entire rows.
> 

Would it be all right to do that as a follow up?

Comment 4

11 years ago
(In reply to comment #3)
> (In reply to comment #2)
> > My reading of the SVG specification is that the union (ie, the old behavior of
> > the region tracking) is what should be tiled, not the whole area.  If that is
> > the case, your changes to add a subregion type seem unnecessary.
> 
> I'm going by http://www.w3.org/TR/SVG/filters.html#FilterPrimitiveSubRegion
> where it talks about feTile using the entire region and not the previous
> filter.

I hate the SVG specification.

> > You are copying individual pixels - it seems with a bit more fun modular math
> > you could copy entire rows.
> 
> Would it be all right to do that as a follow up?

Ok.

Updated

11 years ago
Attachment #258411 - Flags: review?(tor) → review+
(Assignee)

Updated

11 years ago
Attachment #258411 - Flags: superreview?(roc)
(Assignee)

Comment 5

11 years ago
I intend to address the review comment to change "GetSourceRegion" to "GetReferencedRegion" on checkin unless you want to see the change now roc.

Comment on attachment 258411 [details] [diff] [review]
implement tile filter

+                                        DefaultSubregionType defaultSubregionType,

aDefaultSubregionType

+      memcpy(targetData + y * stride + 4 * x,
+             sourceData + tileY * stride + 4 * tileX,
+             4);

Don't use memcpy, use *(PRUint32*)... = *(PRUint32*)...;
Attachment #258411 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 7

11 years ago
Created attachment 273966 [details] [diff] [review]
update to tip

Integrate with lighting filter changes.
Attachment #258411 - Attachment is obsolete: true
(Assignee)

Comment 8

11 years ago
(In reply to comment #7)
> Created an attachment (id=273966) [details]

Oh, and also address first paragraph in comment 2 i.e. s/GetReferencedRegion/GetSourceRegion/

And address sr (comment 6).


Comment 9

11 years ago
Checked in for Robert.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
(Assignee)

Updated

11 years ago
Blocks: 389814
You need to log in before you can comment on or make changes to this bug.