Issue 122600 - [SVG] problems in SvgSvgNode
Summary: [SVG] problems in SvgSvgNode
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: code (show other issues)
Version: 4.0.1-dev
Hardware: All All
: P3 Normal (vote)
Target Milestone: 4.0.1
Assignee: Armin Le Grand
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 121453 122593 122594 122599 122610 122912 122919 122926
  Show dependency tree
 
Reported: 2013-06-26 21:47 UTC by Regina Henschel
Modified: 2014-08-01 13:45 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---
jsc: 4.0.1_release_blocker+


Attachments
Patch to solve several errors with svgsvgnode (41.69 KB, patch)
2013-08-04 19:17 UTC, Regina Henschel
no flags Details | Diff
Updated patch (41.01 KB, patch)
2013-08-05 08:17 UTC, Regina Henschel
rb.henschel: review? (Armin.Le.Grand)
Details | Diff
patch to solve build breaker on Linux 32bit build bot (17.07 KB, patch)
2013-08-15 09:26 UTC, Oliver-Rainer Wittmann
orw: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Regina Henschel 2013-06-26 21:47:52 UTC
I have generated this TASK issue to collect all the problems with SvgSvgNode. These have likely to be solved all together, but it is better for testing the solution, when each problem has it's own description.
Comment 1 Regina Henschel 2013-08-04 19:17:20 UTC
Created attachment 81247 [details]
Patch to solve several errors with svgsvgnode

The patch assumes, that the internal mpViewBox has the parameter meaning x1,y1,x2,y2 and therefore converts the svg meaning x,y,width,height accordingly.

The patch effects only svgio.

The patch corrects several bugs and shortcomings in svgsvgnode. Find some test documents in the linked issues. 

The patch corrects in addition the wrong number type of radius for circle and ellipse.

The patch is large. I suggest to discuss any questions and remarks on the dev-mailing list.
Comment 2 Regina Henschel 2013-08-05 07:15:38 UTC
Change on svgellipsenode is wrong, have to reset that, but fix for svgcirclenode is ok.
Comment 3 Regina Henschel 2013-08-05 08:17:25 UTC
Created attachment 81251 [details]
Updated patch
Comment 4 Armin Le Grand 2013-08-13 15:47:36 UTC
ALG: Checked, only #122575# is not yet solved and needs to betaken out in my opinion. All other stuff works well, compared with firefox as reference.

Well done, Regina!
Esp. using seekReferenceWidth/seekReferenceHeight is pretty much exactly what I would have done, too ;-}

Also checked with all my collected svg bugdocs over time, all work well, no regression visible.
Since this fixes a crash and a regression in AOO400 i request the showstopper flag.
Comment 5 jsc 2013-08-13 15:54:10 UTC
approve showstopper request 

change type to defect because it solves primary a crash
Comment 6 SVN Robot 2013-08-13 17:03:34 UTC
"alg" committed SVN revision 1513591 into trunk:
i122600 Added patch from Regina that fixes a bunch of tasks (list see in task...
Comment 7 SVN Robot 2013-08-13 17:03:35 UTC
"alg" committed SVN revision 1513592 into branches/AOO401:
i122600 Added patch from Regina that fixes a bunch of tasks (list see in task...
Comment 8 Armin Le Grand 2013-08-13 17:04:10 UTC
ALG: Comitted in trunk and AOO401 branch, done.
Comment 9 Oliver-Rainer Wittmann 2013-08-15 09:26:39 UTC
Created attachment 81333 [details]
patch to solve build breaker on Linux 32bit build bot

adjustment to correct compile error on Linux 32bit build bot.

the compile error is:
/home/buildslave20/slave20/openoffice-linux32-nightly/build/main/svgio/source/svgreader/svgsvgnode.cxx: In member function 'virtual const basegfx::B2DRange* svgio::svgreader::SvgSvgNode::getCurrentViewPort() const':
/home/buildslave20/slave20/openoffice-linux32-nightly/build/main/svgio/source/svgreader/svgsvgnode.cxx:636:71: error: taking address of temporary [-fpermissive]
/home/buildslave20/slave20/openoffice-linux32-nightly/build/main/svgio/source/svgreader/svgsvgnode.cxx:679:75: error: taking address of temporary [-fpermissive]
/home/buildslave20/slave20/openoffice-linux32-nightly/build/main/svgio/source/svgreader/svgsvgnode.cxx:698:67: error: taking address of temporary [-fpermissive]
make: *** [/home/buildslave20/slave20/openoffice-linux32-nightly/build/main/solver/400/unxlngi6.pro/workdir/CxxObject/svgio/source/svgreader/svgsvgnode.o] Error 1
Comment 10 Oliver-Rainer Wittmann 2013-08-15 09:27:33 UTC
reopened for adjusting the patched code
Comment 11 Regina Henschel 2013-08-15 09:56:22 UTC
I get in apply
warning: main/svgio/source/svgreader/svgsvgnode.cxx has type 100755, expected 100644
Comment 12 Oliver-Rainer Wittmann 2013-08-15 10:18:02 UTC
(In reply to Regina Henschel from comment #11)
> I get in apply
> warning: main/svgio/source/svgreader/svgsvgnode.cxx has type 100755,
> expected 100644

hm... I am not sure.

I assume that you applied the patch which I attached. Right?
It looks like as the access rights to the file are not the ones which the 'patch' tool is expecting.


BTW, I am changing the issue status back to REOPENED as I think we need to solve the compile error on Linux 32bit build bot. I could the same compile error on my Ubuntu 11.10 32bit VM.
Comment 13 Regina Henschel 2013-08-15 10:41:17 UTC
(In reply to Oliver-Rainer Wittmann from comment #12)
> hm... I am not sure.
> 
> I assume that you applied the patch which I attached. Right?

Yes. I use "git apply".

> It looks like as the access rights to the file are not the ones which the
> 'patch' tool is expecting.
> 
> 
> BTW, I am changing the issue status back to REOPENED as I think we need to
> solve the compile error on Linux 32bit build bot. I could the same compile
> error on my Ubuntu 11.10 32bit VM.

That is OK.

I can apply your patch.
I have tested my test cases with primitives like rectangle, circle and ellipse. That cases pass all well. Need to look at markers now, but that needs some time.
Comment 14 Oliver-Rainer Wittmann 2013-08-15 10:47:23 UTC
(In reply to Regina Henschel from comment #13)
> (In reply to Oliver-Rainer Wittmann from comment #12)
> > 
> > [snip] 
> > 
> 
> I can apply your patch.
> I have tested my test cases with primitives like rectangle, circle and
> ellipse. That cases pass all well. Need to look at markers now, but that
> needs some time.

sounds good.
If all your test cases pass well, I think we should consult apply the patch in order to get a Linux 32bit build bot build. But we should leave this issue open in order to get a review from Armin.


Again ;-) - changing status to REOPENED
Comment 15 Oliver-Rainer Wittmann 2013-08-15 10:48:29 UTC
(In reply to Oliver-Rainer Wittmann from comment #14)
> (In reply to Regina Henschel from comment #13)
> > (In reply to Oliver-Rainer Wittmann from comment #12)
> > > 
> > > [snip] 
> > > 
> > 
> > I can apply your patch.
> > I have tested my test cases with primitives like rectangle, circle and
> > ellipse. That cases pass all well. Need to look at markers now, but that
> > needs some time.
> 
> sounds good.
> If all your test cases pass well, I think we should consult apply the patch
> in order to get a Linux 32bit build bot build. But we should leave this
> issue open in order to get a review from Armin.
> 

ignore word 'consult' - next time I will read my written text twice.
Comment 16 Oliver-Rainer Wittmann 2013-08-21 09:17:33 UTC
(In reply to Oliver-Rainer Wittmann from comment #14)
> (In reply to Regina Henschel from comment #13)
> > (In reply to Oliver-Rainer Wittmann from comment #12)
> > > 
> > > [snip] 
> > > 
> > 
> > I can apply your patch.
> > I have tested my test cases with primitives like rectangle, circle and
> > ellipse. That cases pass all well. Need to look at markers now, but that
> > needs some time.
> 
> sounds good.
> If all your test cases pass well, I think we should apply the patch
> in order to get a Linux 32bit build bot build. But we should leave this
> issue open in order to get a review from Armin.
> 

Private communication with Regina reveals that all test cases went well.
Thus, I will apply the patch on trunk, but not yet on branch AOO401.
Comment 17 SVN Robot 2013-08-21 09:39:22 UTC
"orw" committed SVN revision 1516122 into trunk:
122600: solve access memory problem of <SvgSvgNode::getCurrentViewPort()>
Comment 18 Oliver-Rainer Wittmann 2013-08-21 09:54:11 UTC
Armin:
As one of the experts in this area, could you please have a review of the recently applied patch on trunk?
This patch respectively a revised one should be also applied on branch AOO401. But before a review from your side is much appreciated. Thx in advance.
Comment 19 Armin Le Grand 2013-08-26 13:32:06 UTC
ALG: Checked the patch, looks good. The key is anyways to ask for *.isEmpty at the places where a comparison to NULL was used before. Sorry for having this overlooked, and *argh* for the win compiler not asserting this. Is there a compiler option for the win compiler to do so? If someone knows - this may be worth turning it on, it is hunted by the linux version anyways.
Patch can be applied to OOO401 branch, too.
Comment 20 SVN Robot 2013-08-26 13:53:38 UTC
"alg" committed SVN revision 1517538 into branches/AOO401:
122600: solve access memory problem of <SvgSvgNode::getCurrentViewPort()>
Comment 21 Armin Le Grand 2013-08-26 15:05:05 UTC
ALG: Comitted to AOO401 branch, too.
Comment 22 Oliver-Rainer Wittmann 2013-08-27 09:29:15 UTC
@Regina:
After Armin's review we have got this issue solved on trunk and on branch AOO401.
Could you know check the depending issues and update their status accordingly? Thx in advance.
Comment 23 Regina Henschel 2013-08-28 14:03:36 UTC
Verified all depending issues and set them to fixed.
Comment 24 Oliver-Rainer Wittmann 2013-08-28 14:21:14 UTC
(In reply to Regina Henschel from comment #23)
> Verified all depending issues and set them to fixed.

Cool. Thank you
Comment 25 jsc 2013-08-30 09:02:49 UTC
set target milestone
Comment 26 Kay 2013-09-16 21:54:55 UTC
I can import svg files OK on their own into a text document. I absolutely positively can not import svg files into a frame in a text document. Plain images into a frame are fine. I get error beeps then nothing. I don't know if this is specifically related to this issue. I will search for one better related to frames and svg specifically.

AOO401m4(Build:9713)  -  Rev. 1521921
2013-09-12 07:44 - Linux i686

Leaving as resolved -- probably more related to frames handling import.
Comment 27 Armin Le Grand 2013-09-17 10:05:07 UTC
ALG: @Kay: Thanks for checking.
- Is it possible in AOO341 to 'import svg files into a frame in a text document'? If yes, how exactly to do that (in a new task).
- The beeps should be assertions for unknown SVG tags, a hint that the SVG is interpreted at least...
Comment 28 Regina Henschel 2014-08-01 13:45:54 UTC
All sub-issues solved. Close the task.