Hey Devs,
I've just committed a fix for that darn disappearing text and other items. The culprit was a blank _item_bbox, the code for _item_bbox was very sparse and looking back to the history of where it comes from shows that Niko Kiirala introduced it back in r3505 (2007) to improve filter effect handling when transformed.
What I've done is stripped it out and replaced it with the _bbox used for all the other operations. This means the bounds for the filtering process won't be different for the bounds during other parts of the process. See revision 12528 for the gory details.
What I should be sure of is what Niko was trying to do and if Ted Gould (r6891) or Krzysztof Kosiński (r10347.1.21) spotted anything as they were refactoring the code.
Please do test trunk for issues surrounding filters and transforms. I'd like to make sure fixing this issue doesn't introduce another.
Best Regards, Martin Owens
El 18/09/13 16:27, Martin Owens escribió:
Hey Devs,
I've just committed a fix for that darn disappearing text and other items. The culprit was a blank _item_bbox, the code for _item_bbox was very sparse and looking back to the history of where it comes from shows that Niko Kiirala introduced it back in r3505 (2007) to improve filter effect handling when transformed.
What I've done is stripped it out and replaced it with the _bbox used for all the other operations. This means the bounds for the filtering process won't be different for the bounds during other parts of the process. See revision 12528 for the gory details.
What I should be sure of is what Niko was trying to do and if Ted Gould (r6891) or Krzysztof Kosiński (r10347.1.21) spotted anything as they were refactoring the code.
Please do test trunk for issues surrounding filters and transforms. I'd like to make sure fixing this issue doesn't introduce another.
Best Regards, Martin Owens
After a really quick inspection, everything seems ok. The disappearing blurred text bug is gone, and that alone makes me happy :) The filtered objects don't get dark when transformed anymore. Not sure if it was your fix too. was it?
However, I found something that I'm not sure related to this commit (I can't tell because I didn't try before, please take a look): some of the filters don't work properly. The easiest way to check is to apply a drop shadow filter to a text. It does nothing, although the bounding box changes its dimensions.
Again, I didn't check before your commit, so I don't know if it worked before. Actually I think it's the first time I try filters in this devel cycle :-)
Anyway, good work. That annoying bug was around too much time.
Gez.
On 2013-09-18 23:07 +0200, Guillermo Espertino (Gez) wrote:
El 18/09/13 16:27, Martin Owens escribió:
Hey Devs,
I've just committed a fix for that darn disappearing text and other items. The culprit was a blank _item_bbox, the code for _item_bbox was very sparse and looking back to the history of where it comes from shows that Niko Kiirala introduced it back in r3505 (2007) to improve filter effect handling when transformed.
What I've done is stripped it out and replaced it with the _bbox used for all the other operations. This means the bounds for the filtering process won't be different for the bounds during other parts of the process. See revision 12528 for the gory details.
What I should be sure of is what Niko was trying to do and if Ted Gould (r6891) or Krzysztof Kosiński (r10347.1.21) spotted anything as they were refactoring the code.
Please do test trunk for issues surrounding filters and transforms. I'd like to make sure fixing this issue doesn't introduce another.
Best Regards, Martin Owens
After a really quick inspection, everything seems ok. The disappearing blurred text bug is gone, and that alone makes me happy :) The filtered objects don't get dark when transformed anymore. Not sure if it was your fix too. was it?
However, I found something that I'm not sure related to this commit (I can't tell because I didn't try before, please take a look): some of the filters don't work properly. The easiest way to check is to apply a drop shadow filter to a text. It does nothing, although the bounding box changes its dimensions.
Some findings with recent builds:
r12525: feFlood still works, but the size of the actually used FER has changed (there is now a gap between visual bbox frame and filled area from feFlood, or other filter primitives like e.g. feTurbulance) r12528: feFlood no longer fills the FER with a visible color (this breaks effects like e.g. Drop Shadow, see Gez's report)
r12525: Inkscape crashes on load of this file (r12524 is ok): http://www.w3.org/TR/SVG11/images/filters/enable-background-01.svg reference image: http://www.w3.org/TR/SVG11/images/filters/enable-background-01.png source: http://www.w3.org/TR/SVG11/filters.html#AccessingBackgroundImage
r12525: breaks FER position and size: While trying to figure out the current relationship between FER, visual bounding box (and stroke with), I came across another regression in the filter editor: Changing coordinates and/or dimensions in the filter editor does not live-update the filter output area (it only affects the position and size of the visual bbox frame). This seems to have started with the changes in r12362 (in tests with archived builds it works as expected with r12359, in r12364 and later revisions one has to force a refresh or reload the file to see the FER updated), and regressed one more step in r12525 (now AFAICT the FER neither is correctly updated when forcing a refresh nor when reloading the file).
I've committed a couple of fixes for the reported issues for 12528 and 12525. I'm not sure if I caught the one relating to stroke any examples might be really useful. See r12536
Martin,
On 2013-09-19 14:35 +0200, Martin Owens wrote:
I've committed a couple of fixes for the reported issues for 12528 and 12525. I'm not sure if I caught the one relating to stroke any examples might be really useful. See r12536
Testing latest trunk (r12544) build:
On 2013-09-19 07:23 +0200, su_v wrote:
r12525: Inkscape crashes on load of this file (r12524 is ok): http://www.w3.org/TR/SVG11/images/filters/enable-background-01.svg reference image: http://www.w3.org/TR/SVG11/images/filters/enable-background-01.png source: http://www.w3.org/TR/SVG11/filters.html#AccessingBackgroundImage
No more crash on load with r12544, but renders incorrectly (with r12524 it looked the same as the reference image): https://www.dropbox.com/sh/utaa2r0r82ld9zb/EUzH6YpyCe#/
r12525: breaks FER position and size: While trying to figure out the current relationship between FER, visual bounding box (and stroke with), I came across another regression in the filter editor: Changing coordinates and/or dimensions in the filter editor does not live-update the filter output area (it only affects the position and size of the visual bbox frame). This seems to have started with the changes in r12362 (in tests with archived builds it works as expected with r12359, in r12364 and later revisions one has to force a refresh or reload the file to see the FER updated), and regressed one more step in r12525 (now AFAICT the FER neither is correctly updated when forcing a refresh nor when reloading the file).
AFAICT still broken with r12544.
Maybe one of the SVG-filter-effects experts (wrt the spec and inkscape's implementation) can test and comment?
On 2013-09-19 23:39 +0200, su_v wrote:
r12525: breaks FER position and size: While trying to figure out the current relationship between FER, visual bounding box (and stroke with), I came across another regression in the filter editor: Changing coordinates and/or dimensions in the filter editor does not live-update the filter output area (it only affects the position and size of the visual bbox frame). This seems to have started with the changes in r12362 (in tests with archived builds it works as expected with r12359, in r12364 and later revisions one has to force a refresh or reload the file to see the FER updated), and regressed one more step in r12525 (now AFAICT the FER neither is correctly updated when forcing a refresh nor when reloading the file).
AFAICT still broken with r12544.
Maybe one of the SVG-filter-effects experts (wrt the spec and inkscape's implementation) can test and comment?
Test case uploaded here: https://www.dropbox.com/sh/texl8xxvw77pkjo/LY_NVrWLGp?lst#/
Files: FER-regression-4-batik17.png : reference rendering Squiggle (Batik 1.7)
FER-regression-4-chromium.png : screenshot chromium 31.0.1638.0 (224352)
FER-regression-4-r12359.png : export inkscape r12359 (same as batik & chromium)
FER-regression-4-r12364.png : export inkscape r12364 (visual instead of geometric bounds for FER?)
FER-regression-4-r12524.png : export inkscape r12524 (same as with r12364)
FER-regression-4-r12550.png : export inkscape r12525 (what happened?)
FER-regression-4.svg : sample SVG file
On Fri, 2013-09-20 at 16:12 +0200, su_v wrote:
Test case uploaded here: https://www.dropbox.com/sh/texl8xxvw77pkjo/LY_NVrWLGp?lst#/
Thank you SUV, these tests and images are of great help in researching this code more.
Clearly a blunderbuss was the worst kind of scalpel for this bug. Even though most of the code is actually back in ~550 the filters are very delicate and I think I'll go through them piece by piece again. Last time was hours of removing lines one at a time and recompiling.
Martin,
Some comments which might help you understand what is going on in the filter code:
The interactive Cairo renderer uses tiling: only one strip of the image is rendered at a time. The SVG specification does not have special provisions for this, and filter rendering is described in terms of operations on pixel buffers that render the entire object at once. Therefore, FER (filter effect region) is well defined in SVG, and corresponds by default to 120% of the width and height of the source object. FER is returned by the function filter_effect_area().
To implement tiling, the function area_enlarge() is used to compute the "dependency area" of the filter - the pixels which must be rendered in order to display the requested tile correctly. This is somewhat wasteful, because for filters effects that contain large dependency regions, we computing lots of pixels but end up using only one tile. There is no perfect solution to this, because allocating a buffer for the entire object is not possible at large zoom levels, though I guess we can do better than at present.
Calling area_enlarge() on Gaussian blur filter will return a bounding box which is expanded by around 2.4 times the standard deviation of the Gaussian blur and clipped to the FER.
I hope this clears up things a bit.
Regards, Krzysztof
OK
Thanks for everyone's help on this bug.
The crux of it was: The libnrtype/Layout-NG-Output.cpp:185 was setting the visual area for the filter to the inner text drawing, instead of the outer arena where the filter was applied. Thus the filter would often be unset, or worse zeroed by sp-item.cpp as the node got moved around. It would also aquire previous coords as the updates from LayoutNG were going nowhere.
The rest of the modified code from 25 and 28 is reverted. SUVs test passes, drop shadows work and text behaves correctly.
There was one extra niggle, the text visualBounds didn't have the filter area expanded like the sp-item. So I had to have an intermediary step for that to be set and enlarged.
Please test r12556.
Martin,
On Fri, 2013-09-20 at 18:06 +0200, Krzysztof Kosiński wrote:
Some comments which might help you understand what is going on in the filter code:
The interactive Cairo renderer uses tiling: only one strip of the image is rendered at a time. The SVG specification does not have special provisions for this, and filter rendering is described in terms of operations on pixel buffers that render the entire object at once. Therefore, FER (filter effect region) is well defined in SVG, and corresponds by default to 120% of the width and height of the source object. FER is returned by the function filter_effect_area().
To implement tiling, the function area_enlarge() is used to compute the "dependency area" of the filter - the pixels which must be rendered in order to display the requested tile correctly. This is somewhat wasteful, because for filters effects that contain large dependency regions, we computing lots of pixels but end up using only one tile. There is no perfect solution to this, because allocating a buffer for the entire object is not possible at large zoom levels, though I guess we can do better than at present.
Calling area_enlarge() on Gaussian blur filter will return a bounding box which is expanded by around 2.4 times the standard deviation of the Gaussian blur and clipped to the FER.
I hope this clears up things a bit.
Regards, Krzysztof
On 2013-09-18 21:27 +0200, Martin Owens wrote:
I've just committed a fix for that darn disappearing text and other items. The culprit was a blank _item_bbox, the code for _item_bbox was very sparse and looking back to the history of where it comes from shows that Niko Kiirala introduced it back in r3505 (2007) to improve filter effect handling when transformed.
What I've done is stripped it out and replaced it with the _bbox used for all the other operations. This means the bounds for the filtering process won't be different for the bounds during other parts of the process. See revision 12528 for the gory details.
What I should be sure of is what Niko was trying to do and if Ted Gould (r6891) or Krzysztof Kosiński (r10347.1.21) spotted anything as they were refactoring the code.
Please do test trunk for issues surrounding filters and transforms. I'd like to make sure fixing this issue doesn't introduce another.
Regression introduced with r12528: - Bug #1229573: trunk: broken rendering of gradients with "objectBoundingBox" as gradient units (rev >= 12528) https://bugs.launchpad.net/inkscape/+bug/1229573
2013/10/1 su_v <suv-sf@...58...>:
Regression introduced with r12528:
- Bug #1229573: trunk: broken rendering of gradients with "objectBoundingBox" as gradient units (rev >= 12528) https://bugs.launchpad.net/inkscape/+bug/1229573
The changes in r12528 related to _item_bbox were wrong. I reverted them in r12648.
The real culprit in all these cases was not anything in the display tree, but the incorrect update order in the SP tree. I fixed all SP objects which updated their children so that they now do this in the correct order.
Regards, Krzysztof
participants (4)
-
Guillermo Espertino (Gez) -
Krzysztof Kosiński -
Martin Owens -
su_v