wxSVGFileDC improvements. #215

MaartenBent · 2016-02-14T17:52:31Z

I used the screens in the drawing sample and tried to fix most of the issues when saving them as SVG, including:

Enabled usage of clipping regions.
Correctly draw polypolygons.
Draw lines as one long line instead of many short lines.
Drawing text improvements (position, multi-line, underlined, strike-through).
Support more brush and pen styles.
Add Saving as SVG to drawing sample.
Implemented Clear().
Set the SVG title.
Produce valid svg/xml.
Correctly draw ellipses and arcs.

The commit messages have some more info.

There are a lot of commits to make the individual changes more obvious. I don't have any objection to squashing them together.

Screenshots as wxDC, SVG (old), SVG (new). I converted the SVGs to PNGs, hence the lower quality.

Brushes:

Lines:

Polygon:

Regions:

Text:

Circles:

SVG Arc Example:

MaartenBent · 2016-02-15T22:06:47Z

I had to remove the title attribute for images. It is possible to add a title using <title>text</title> inside the image tag, but I don't see any benefits of adding the current (fixed) title.

crnd · 2016-03-01T15:44:39Z

include/wx/dcsvg.h

@@ -66,7 +66,8 @@ class WXDLLIMPEXP_CORE wxSVGFileDCImpl : public wxDCImpl
 {
 public:
    wxSVGFileDCImpl( wxSVGFileDC *owner, const wxString &filename,
-                     int width=320, int height=240, double dpi=72.0 );
+                     int width=320, int height=240, double dpi=72.0,


Should try to keep the code style consistent by using spaces around the assignment operator:

int width = 320, int height = 240, double dpi = 72.0

MaartenBent · 2016-03-04T20:49:48Z

Some modifications:

Use spaces around assignment operator
Use switch instead of if else if else if else for pen/brush styles
Split multi-line text and font changes into separate commits.

The text alignment was already fixed in the original code but I accidentally removed it when adding support for multi-line text. Commit below restores the original alignment fix using the text descent.

vadz · 2016-03-05T01:55:24Z

include/wx/dcsvg.h

@@ -66,7 +66,8 @@ class WXDLLIMPEXP_CORE wxSVGFileDCImpl : public wxDCImpl
 {
 public:
    wxSVGFileDCImpl( wxSVGFileDC *owner, const wxString &filename,
-                     int width=320, int height=240, double dpi=72.0 );
+                     int width = 320, int height = 240, double dpi = 72.0,
+                     const wxString &title = wxT("") );


Hardly very important but it's better to use = wxString() for the default value (in spite of the fact that most of the existing code doesn't do it).

vadz · 2016-03-05T02:03:03Z

Thanks, this definitely looks useful and it's great to have better SVG output support. If we could avoid adding more duplicated code (and I know there is already plenty of it), it would be really perfect...

MaartenBent · 2016-03-07T23:04:58Z

I addressed your comments in the new commits.

The title is metadata, just like html and pdf can have a title. If you open a svg file in a browser, the title is shown as tab-name.

wxSVGFileDC takes ownership of the bitmap handler according to the documentation, so it should also take care of deleting it. I changed the usage of pointers to smart pointers to simplify this.

default: is needed to prevent compilers from complaining about missing enumeration values:
warning: enumeration value 'xxx' not handled in switch [-Wswitch]

If desired, I can rebase and cleanup some commits (as some changes have been replaced by newer commits).

vadz · 2016-03-12T17:27:08Z

src/common/dcsvg.cpp

+    float w = pen.GetWidth();
+    if (pen.GetWidth() == 0)
+        w = 1;
+    w = w / 3;


I wonder where do all these factors (3 here, 2, 5, 8, 10 below) come from? It would be nice to have a comment explaining it...

vadz · 2016-03-12T17:36:10Z

Sorry, I still have a few minor remarks, it would be great if you could please address them. Thanks again!

MaartenBent · 2016-03-15T23:12:13Z

Updated. Waiting for input on removing pointer of m_outfile (see outdated comment), or if last commit is sufficient.

Relevant switch statements now include all enums. ~~But I wonder if clang will complain about missing default case, or about empty statements (and wxFALLTHROUGH is needed).~~ _{^{Appearantly not (only with -Weverything?).}}

a-wi · 2016-06-09T18:22:48Z

samples/drawing/drawing.cpp

+#endif
+            wxSVGFileDC svgdc(dlg.GetPath(), width, height, 72, wxT("Drawing sample"));
+            svgdc.SetBitmapHandler(new wxSVGBitmapEmbedHandler());
+            m_canvas->Draw(svgdc);


This trick with swapping DC's doesn't work if "graphics screen" is displayed. In this case an empty image is saved and artefacts are displayed on the screen. It would be good to disable somehow the option to "save as SVG image" if graphics screen is active.

It was implemented 3 and half years ago in 614e38d. Close remaining clipping regions when closing the SVG file.

Each polygon in the poly-polygon needs to end with the start-point. The implementation is similar to the wxDCImpl implementation, except for the extra end points that are inserted.

Do not draw each line separately because the lines will not be connected. E.g. when editing a SVG, moving a point should also move the connecting lines.

Support underlined and strike-through text. Set the text length (see http://trac.wxwidgets.org/ticket/17271). Preserve leading and trailing white-space. Use wxS macro consistently. Closes wxWidgets#17271.

Draw a rectangle with the background colour and a transparent pen. Closes wxWidgets#15788.

Use the correct doc-type and specify the encoding. 'title' is not a valid attribute of <image> so remove it. Removed superfluous white-space and improved indenting in generated XML.

Changed the colour of the third star-polygon to test brush pattern with different colours. Use the width and height of the scroll panel as image size.

Override SetDeviceOrigin, SetLogicalOrigin and SetAxisOrientation from wxDC and mark the graphics as changed, so the correct transform translations are applied.

Ellipses with the same start and end point (circles) where not drawn because the angle becomes 0 degrees. Fixed by drawing two half circles. Do not close ellipses if a transparent brush is used (to match wxDC behavior). See issue wxWidgets#17557.

For some combinations of start and end angles, the wrong large-arc-flag was calculated. Fixed by correctly converting the wxDC angles to SVG angles (shift -90 degrees, and invert to clockwise direction). Arcs with the same start and end point (circles) where not drawn because the angle becomes 0 degrees. Fixed by drawing two half circles. Elliptic arcs with a non-transparent brush had an extra line from the center to the start point of the arc. Fixed by first drawing the arc without border, then only the border. Arcs with small angles would become invisible because the start and end point map to the same (integer) coordinate. Very large arcs would be distorted because the start and end point coordinates did not line up. Using floating point values resolves this. See issue wxWidgets#17557.

Consistent white-space usage. Use wxS macros for strings. Check if output stream is OK when writing. Removed unnecessary Borland pragmas.

MaartenBent · 2016-06-12T16:45:45Z

I updated the commits to use wxS.
Removed all unnecessary white space from generated XML and added some indenting.
Updated the sample to handle graphics screen.

Added an extra commit to clean-up the source code and use wxS instead of wxT.

vadz · 2016-06-13T02:21:48Z

If there are no more comments/remarks/objections, I'm going to merge it soon.

Thanks!

…Bent/wxWidgets Many improvements in wxSVGFileDC to improve its support of wxDC API including: - Enabled usage of clipping regions. - Correctly draw polypolygons. - Draw lines as one long line instead of many short lines. - Drawing text improvements (position, multi-line, underlined, strike-through). - Support more brush and pen styles. - Add Saving as SVG to drawing sample. - Implemented Clear(). - Set the SVG title. - Produce valid svg/xml. - Correctly draw ellipses and arcs. See #215

wnitzan · 2019-03-22T18:24:51Z

The next wxPython release will still pull from WX_3_0_BRANCH.
Will it be possible for you to back-port these SVG changes into WX_3_0_BRANCH?

MaartenBent · 2019-03-22T23:53:01Z

Sure, I can create a PR for 3.0 branch with all recent SVG changes.

After comparing the headers I do have some concerns about ABI/API compatibility. I'll address them in the new PR.

wnitzan · 2019-03-23T00:06:59Z

I can't thank you enough. These SVG fixes are badly needed on the wxPython side.
Please come back to update here with your efforts, so I can follow your progress.

MaartenBent force-pushed the wxsvgfiledc-improvements branch from 423520f to 488e15f Compare February 15, 2016 22:00

crnd reviewed Mar 1, 2016
View reviewed changes

MaartenBent force-pushed the wxsvgfiledc-improvements branch from 488e15f to 6653e8e Compare March 4, 2016 20:44

MaartenBent force-pushed the wxsvgfiledc-improvements branch from 97b16fd to a243ec4 Compare March 4, 2016 22:53

vadz reviewed Mar 5, 2016
View reviewed changes

vadz modified the milestone: 3.1.1 Mar 5, 2016

vadz reviewed Mar 12, 2016
View reviewed changes

MaartenBent force-pushed the wxsvgfiledc-improvements branch from b9914ce to 9552250 Compare March 15, 2016 23:07

MaartenBent force-pushed the wxsvgfiledc-improvements branch from 9552250 to c50fd98 Compare March 15, 2016 23:22

MaartenBent force-pushed the wxsvgfiledc-improvements branch 5 times, most recently from 7d43f36 to 1e78471 Compare June 4, 2016 16:28

a-wi reviewed Jun 9, 2016
View reviewed changes

MaartenBent added 16 commits June 12, 2016 15:59

Enable usage of clipping regions in wxSVGFileDC.

ebab640

It was implemented 3 and half years ago in 614e38d. Close remaining clipping regions when closing the SVG file.

Correctly draw poly-polygons in wxSVGFileDC.

444c5fd

Each polygon in the poly-polygon needs to end with the start-point. The implementation is similar to the wxDCImpl implementation, except for the extra end points that are inserted.

Draw lines in wxSVGFileDC as one connected line.

5084c6d

Do not draw each line separately because the lines will not be connected. E.g. when editing a SVG, moving a point should also move the connecting lines.

Support multi-line text in wxSVGFileDC.

b4c9927

Improvements to printing text in wxSVGFileDC.

b424445

Support underlined and strike-through text. Set the text length (see http://trac.wxwidgets.org/ticket/17271). Preserve leading and trailing white-space. Use wxS macro consistently. Closes wxWidgets#17271.

Implemented Clear in wxSVGFileDC.

3cc4d51

Draw a rectangle with the background colour and a transparent pen. Closes wxWidgets#15788.

Support more pen styles in wxSVGFileDC.

4641710

Support more brush styles in wxSVGFileDC.

2171d40

Allow to set the SVG title when creating a wxSVGFileDC.

045265a

Generate valid XML in wxSVGFileDC and updated wxSVGVersion.

b55a18f

Use the correct doc-type and specify the encoding. 'title' is not a valid attribute of <image> so remove it. Removed superfluous white-space and improved indenting in generated XML.

Add saving as SVG to drawing sample.

c7e4b30

Changed the colour of the third star-polygon to test brush pattern with different colours. Use the width and height of the scroll panel as image size.

Improved memory management in wxSVGFileDC.

1717db0

Support SetDeviceOrigin and SetAxisOrientation in wxSVGFileDC.

9e07ba8

Override SetDeviceOrigin, SetLogicalOrigin and SetAxisOrientation from wxDC and mark the graphics as changed, so the correct transform translations are applied.

Code cleanup in wxSVGFileDC.

97c7ac4

Consistent white-space usage. Use wxS macros for strings. Check if output stream is OK when writing. Removed unnecessary Borland pragmas.

MaartenBent force-pushed the wxsvgfiledc-improvements branch from 1e78471 to 97c7ac4 Compare June 12, 2016 16:42

vadz merged commit 97c7ac4 into wxWidgets:master Jun 13, 2016

MaartenBent deleted the wxsvgfiledc-improvements branch June 14, 2016 18:16

MaartenBent mentioned this pull request Mar 24, 2019

Apply wxSVGFileDC changes to 3.0 branch #1275

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wxSVGFileDC improvements. #215

wxSVGFileDC improvements. #215

MaartenBent commented Feb 14, 2016 •

edited

MaartenBent commented Feb 15, 2016

crnd Mar 1, 2016

MaartenBent commented Mar 4, 2016

vadz Mar 5, 2016

vadz commented Mar 5, 2016

MaartenBent commented Mar 7, 2016

vadz Mar 12, 2016

vadz commented Mar 12, 2016

MaartenBent commented Mar 15, 2016

a-wi Jun 9, 2016

MaartenBent commented Jun 12, 2016

vadz commented Jun 13, 2016

wnitzan commented Mar 22, 2019

MaartenBent commented Mar 22, 2019

wnitzan commented Mar 23, 2019

wxSVGFileDC improvements. #215

wxSVGFileDC improvements. #215

Conversation

MaartenBent commented Feb 14, 2016 • edited

MaartenBent commented Feb 15, 2016

crnd Mar 1, 2016

Choose a reason for hiding this comment

MaartenBent commented Mar 4, 2016

vadz Mar 5, 2016

Choose a reason for hiding this comment

vadz commented Mar 5, 2016

MaartenBent commented Mar 7, 2016

vadz Mar 12, 2016

Choose a reason for hiding this comment

vadz commented Mar 12, 2016

MaartenBent commented Mar 15, 2016

a-wi Jun 9, 2016

Choose a reason for hiding this comment

MaartenBent commented Jun 12, 2016

vadz commented Jun 13, 2016

wnitzan commented Mar 22, 2019

MaartenBent commented Mar 22, 2019

wnitzan commented Mar 23, 2019

MaartenBent commented Feb 14, 2016 •

edited