Created attachment 122531 [details] matrix: parents combined with text of different fills +++ This bug was initially created as a clone of Bug #97542 +++ Steps: 1) Open Writer 2) Open attached text_with_parent_fills.svg 3) Compare the output with what you can find in your browser or inkscape and see differences currently in row 3 the columns 4 to 6 are imported wrong (text is filled with parent fill instead of the plain set in text) patch in bug #97542 fixes row 3, column 4 and 5 in row 6, column 4 to 5 is wrongly imported, text is filled with parent fill instead of pattern shapes instead of text got similar problems and strokes got similar problems with parents too got draft to fix this but needs some testing
Confirmed. Version: 5.2.0.0.alpha0+ Build ID: fea95da81260bc7eabe7ece595829009b2db3e62 CPU Threads: 2; OS Version: Linux 4.2; UI Render: default; TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2016-02-10_01:41:22 Locale: en-US (en_US.UTF-8) @JoNi: Does Xisco's fix in bug 97542 not fix this?
> @JoNi: Does Xisco's fix in bug 97542 not fix this? not all cases, only fixes case when parent has gradient fill and text has plain colour fill fill and strokes attributes can have types (paints): * plain colour fill * gradient fill * pattern fill but only ONE of them getFill, getStroke, getSvgGradientNodeFill, getSvgGradientNodeStroke, getSvgPatternNodeFill and getSvgPatternNodeStroke return values from parents even if the current node has a different kind of paint is set e.g. getSvgPatternNodeFill should not return a pattern fill from a parent if current element has already a plain colour fill or gradient fill as in this example: <pattern id="svg_pat" x="0" y="0" width="0.5" height="0.5"> <rect x="0" y="0" width="16" height="16" stroke="#00FFFF" /> <circle stroke="#FF0000" cx="13" cy="13" r="8" /> </pattern> <g fill="url(#svg_pat)"> <text fill="#0f0f0f">Text</text> <!-- add some more elements --> </g>
Hi Joni, Do you want me to take a look at it or you're working on it?
(In reply to Xisco Faulí from comment #3) > Hi Joni, Do you want me to take a look at it or you're working on it? It's on gerrit https://gerrit.libreoffice.org/22273 It fixes the cases with text and all kinds of fills (see attachment 122531 [details]), strokes work too. Should do the same for shapes. I still want to look into cases where fill or stroke is defined in css.
Created attachment 122765 [details] matrix: parents combined with text of different strokes
Created attachment 122766 [details] matrix: parents combined with shapes of different fills
Created attachment 122767 [details] matrix: parents combined with text of different strokes
it's always the same issue with fill and stroke attributes. a plain colour is is overwritten by a parent gradient or pattern and a pattern is overwritten by a parent gradient. the attachments demonstrate the behaviour with the text content <text> but it should also apply to <altGlyph>, <textPath>, <tref> and <tspan> and the illicit behaviour on shapes is demonstrated with <rect> but should also apply to <path>, <circle>, <ellipse>, <line>, <polyline> and <polygon>. @Xisco Faulí you suggested unit tests on gerrit, which cases should we test? the 3 fixed cases apply to fill and stroke, 5 different text elements and 7 different shapes. I would like get the change pushed and make a follow up commit for tests.
Jochen Nitschke committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=423b79e7366203db3f57dea75b8cb9eb852b5614 tdf#97752 SVGIO ignore not matching parent paints It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Hi Joni, as you can see, I've already submitted the patch to master. The more unittest we create, the better, but at least it would be nice to do an unittest for every 'else if' you modified.