Download it now!
Bug 97752 - INSERT: SVG problems fill attributes in groups
Summary: INSERT: SVG problems fill attributes in groups
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
5.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: JoNi
URL:
Whiteboard: target:5.2.0
Keywords: filter:svg
Depends on: 97542
Blocks: SVG-Import
  Show dependency treegraph
 
Reported: 2016-02-11 13:55 UTC by JoNi
Modified: 2016-10-25 19:07 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
matrix: parents combined with text of different fills (5.82 KB, image/svg+xml)
2016-02-11 13:55 UTC, JoNi
Details
matrix: parents combined with text of different strokes (6.35 KB, image/svg+xml)
2016-02-18 11:53 UTC, JoNi
Details
matrix: parents combined with shapes of different fills (6.25 KB, image/svg+xml)
2016-02-18 11:54 UTC, JoNi
Details
matrix: parents combined with text of different strokes (6.35 KB, image/svg+xml)
2016-02-18 11:54 UTC, JoNi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description JoNi 2016-02-11 13:55:55 UTC
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
Comment 1 Yousuf Philips (jay) (retired) 2016-02-12 12:08:33 UTC
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?
Comment 2 JoNi 2016-02-14 17:40:47 UTC
> @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>
Comment 3 Xisco Faulí 2016-02-15 16:26:02 UTC
Hi Joni, Do you want me to take a look at it or you're working on it?
Comment 4 JoNi 2016-02-15 21:51:46 UTC
(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.
Comment 5 JoNi 2016-02-18 11:53:18 UTC
Created attachment 122765 [details]
matrix: parents combined with text of different strokes
Comment 6 JoNi 2016-02-18 11:54:06 UTC
Created attachment 122766 [details]
matrix: parents combined with shapes of different fills
Comment 7 JoNi 2016-02-18 11:54:32 UTC
Created attachment 122767 [details]
matrix: parents combined with text of different strokes
Comment 8 JoNi 2016-02-18 12:20:00 UTC
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.
Comment 9 Commit Notification 2016-02-18 15:24:14 UTC
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.
Comment 10 Xisco Faulí 2016-02-18 15:30:23 UTC
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.