603584 - Filter feDisplacementMap broken

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[jonathan chetwynd]

fill in ff nightly predominantly light
Opera and Safari dark

Comment 1

[jonathan chetwynd]

Attachment: os x testcase

Comment 2

[jonathan chetwynd]

Attachment: screengrab, Opera on left, ff on right

Comment 3

[jonathan chetwynd]

url provides use case, or broader context

Comment 4

[Robert Longson [:longsonr]]

How have you established that Firefox is wrong and Opera/Safari is right other than it being 2 against 1?

Comment 5

[Robert Longson [:longsonr]]

Needs a reduced testcase.

Comment 6

[jonathan chetwynd]

Please could you define a little better what you would expect in the testcase?
How would you like me to establish this, other than by filing a bug or taking to the working group?

in my tests before filing a bug, I ascertained that all three browsers closely agreed, when either no filter was applied, or feTurbulence only was applied.

The original fill is dark, and similar to that in Opera and Safari in the final result.

Comment 7

[jonathan chetwynd]

Attachment: testcase

Comment 8

[Robert Longson [:longsonr]]

(In reply to comment #6)
> Please could you define a little better what you would expect in the testcase?
> How would you like me to establish this, other than by filing a bug or taking
> to the working group?

If you think feDisplacementMap is wrong then that should be the only filter. The output of feTurbulence is complicated and so it makes the displacement map data complicated.

Don't use gradients in tests about filters. Don't use more than one rect with the filter on it.

> The original fill is dark, and similar to that in Opera and Safari in the final
> result.

Why are they correct? If you make the testcase simple enough we should be able to work out where the colours are coming from. With feTurbulence we can't really as the exact pixel colours of the turbulence vary so much across the surface and are not directly accessible.

Comment 9

[Robert Longson [:longsonr]]

OK I wrote that before comment 7. Just the turbulence to remove now.

Comment 10

[Robert Longson [:longsonr]]

Alternatively, see bug 584322. Can you figure out why this example (https://bugzilla.mozilla.org/attachment.cgi?id=462849) which also uses turbulence and displacement map now gives the same result as Opera whilst yours does not. If you adjust your filter attributes till you find what makes the difference we may get somewhere.

Comment 11

[jonathan chetwynd]

Attachment: testcase

Comment 12

[jonathan chetwynd]

Attachment: testcase

Comment 13

[jonathan chetwynd]

#10 Robert, this uses  the identical  filter to 584322,
which is  hard to credit...

but at least on OS X the hue is lighter, whereas darker in Opera & Safari.

the only action I  took was to  widen  the  rectangle as when the scale="50"
the  distortion is great for a narrow rectangle.

Comment 14

[jonathan chetwynd]

Attachment: Screngrab: Opera on left ff on right,

Comment 15

[jonathan chetwynd]

4.0b8pre os x 10.6.4

Comment 16

[Robert Longson [:longsonr]]

It all comes down to whether the output of feDisplacementMap should be premultiplied or not.

The specification clearly says how to treat the inputs (in1 and in2) http://www.w3.org/TR/SVG/filters.html#feDisplacementMapElement it does not say how to treat the output. Should that be affected by color-interpolation-filters or should it always be SRGB, LINEARRGB or whatever in1 is or even whatever in2 is.

Firefox takes the output as being whatever colour space is defined by color-interpolation-filters i.e. if in1 starts as LINEARRGB then it finishes as LINEARRGB. It seems like Opera and Safari assume it is SRGB.

This needs taking up with the w3c as it is unclear what is correct.

Comment 17

[jonathan chetwynd]

http://www.w3.org/Bugs/Public/show_bug.cgi?id=11015

Comment 18

[Robert Longson [:longsonr]]

Attachment: patch

in1 is the source image and in2 supplies the displacements. So it does not make sense that applying in2 should change the colour space of the output. The output colour space should be whatever in1 is.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Giving OperatesOnSRGB a side effect and relying on it being called with aInput==0 before aInput==-1 is really ugly. Can we do this in a better way?

Comment 20

[Robert Longson [:longsonr]]

Attachment: how about this instead then?

Comment 21

[Robert Longson [:longsonr]]

Attachment: without the no-op line this time

Comment 22

[Robert Longson [:longsonr]]

Comment on attachment 485497 [details] [diff] [review]
without the no-op line this time

still think I can make this neater

Comment 23

[Robert Longson [:longsonr]]

Comment on attachment 485497 [details] [diff] [review]
without the no-op line this time

I tried restructuring it again without success so this is the best I've come up with.

Comment 24

[Robert Longson [:longsonr]]

Plan c) would be to create a new nsTArray of pointers to images. Then pass that through to the GetxxxColorMap methods. GetOutputColorMap would then work as it would have the in1 image. However this really isn't required if we keep the code in nsSVGFilterInstance to use the in1 image as the output colour map.

Comment 25

[Robert Longson [:longsonr]]

Comment on attachment 485497 [details] [diff] [review]
without the no-op line this time

straightforward patch with reftest.

Comment 26

[Robert Longson [:longsonr]]

Attachment: hg changeset patch

Comment 27

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/49e5daf139f7

Comment 28

[jonathan chetwynd]

is this fix in the trunk?

the latest nightly dated today's date, remains predominantly light, as opposed to dark.

Comment 29

[Dão Gottwald [:dao]]

The fix will be in the next nightly.