Add wxGraphicsContext support to wxSVGFileDC (PR #26395)
Blake-Madden
By calling GetGraphicsContext() on a wxSVGFileDC, you can now access a wxSVGGraphicsContext (derived from wxGraphicsContext) which supports:
- Cubic and quadratic Bézier curves via
wxGraphicsPath - Linear and radial gradient brushes
- Semi-transparent colors and alpha blending
- Arbitrary affine transformations
Basically, the wxSVGGraphicsContext* writes to its parent wxSVGFileDC's SVG buffer as you issue drawing commands to it.
I added a demonstration to the SVG sample (basic stuff, but shows off the features).
The following images are of my output SVG. I'm able to get some nice effects, such as a sunflower:
image.png (view on web)Or a Chernoff face:
image.png (view on web)I believe the full graphics context API is available in here now (at least the parts that are analogous to SVG).
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26395
Commit Summary
- 71c59c2 Initial support for Graphics Context for wxSVGDCFile
- 97edd61 Add Begin/EndLayer support to wxSVGFileDC
- 29f3265 Add gradient & radial pens and brushes to wxSVGGraphicsContext
- dc99f6e In sample, use paint event handle
- 61d08b0 Demonstrate new features in the sample
- 29af5ab Make members static, make helpers private again
- a27c98d Add API docs
- a694fcc Fix version numbers
- 081d479 Implement GetClipBox()
- 2a461ca Minor doc cleanup
File Changes
(18 files)
- M Makefile.in (21)
- M build/bakefiles/files.bkl (2)
- M build/cmake/files.cmake (2)
- M build/files (2)
- M build/msw/makefile.gcc (28)
- M build/msw/makefile.vc (28)
- M build/msw/wx_core.vcxproj (2)
- M build/msw/wx_core.vcxproj.filters (6)
- M build/osx/wxcocoa.xcodeproj/project.pbxproj (8)
- M build/osx/wxiphone.xcodeproj/project.pbxproj (4)
- M include/wx/dcsvg.h (76)
- A include/wx/svggc.h (441)
- M interface/wx/dcsvg.h (51)
- A interface/wx/svggc.h (414)
- M samples/svg/svgtest.cpp (288)
- M src/common/dcsvg.cpp (1032)
- M src/common/descrip.mms (5)
- A src/common/svggc.cpp (1448)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26395.patch
- https://github.com/wxWidgets/wxWidgets/pull/26395.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- ac283b7 Fix indentation that was causing GCC warning/error
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks, this definitely looks very nice and useful!
I didn't have the time to look at the new code yet (but I will), however I do have to ask: why use such strange API instead of adding wxSVGFileGC deriving from wxGraphicsContext as I'd naively expect? Is it significantly simpler to implement in this way?
> + }
+#if wxUSE_GRAPHICS_CONTEXT
+ case Page_SVGGraphicsContext:
+ {
+ wxGraphicsContext* const gc = dc.GetGraphicsContext();
+ if ( !gc )
+ break;
+
+ // Cyan rectangle with red border.
+ gc->SetPen(*wxRED_PEN);
+ gc->SetBrush(*wxCYAN_BRUSH);
+ gc->DrawRectangle(dc.FromDIP(10), dc.FromDIP(10),
+ dc.FromDIP(100), dc.FromDIP(70));
+
+ // Transparent rounded rectangle.
+ gc->SetBrush(wxBrush("DARK ORCHID", wxBRUSHSTYLE_TRANSPARENT));
Just curious: why give a colour to a transparent brush?
> + // Open a group with the given opacity. + void BeginLayer(double opacity); + + // Close the group opened by the most recent BeginLayer() call. + void EndLayer(); +
Should we have a RAII helper calling {Begin,End}Layer() in its ctor/dtor as usual?
> wxString GetSVGDocument() const;
bool Save();
private:
+
+ // String helpers used by both the DC and the SVG graphics context.
An alternative to putting them here would be to put them in include/wx/private/svg.h (maybe in some wxSVG namespace to avoid name clashes).
There is no real problem with doing it like you did, but having them in a completely private header would reduce rebuilds when changing anything here.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 7ef8cfb Improve GetTextExtent precision
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- bd8a69c Just use transparent brush
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + }
+#if wxUSE_GRAPHICS_CONTEXT
+ case Page_SVGGraphicsContext:
+ {
+ wxGraphicsContext* const gc = dc.GetGraphicsContext();
+ if ( !gc )
+ break;
+
+ // Cyan rectangle with red border.
+ gc->SetPen(*wxRED_PEN);
+ gc->SetBrush(*wxCYAN_BRUSH);
+ gc->DrawRectangle(dc.FromDIP(10), dc.FromDIP(10),
+ dc.FromDIP(100), dc.FromDIP(70));
+
+ // Transparent rounded rectangle.
+ gc->SetBrush(wxBrush("DARK ORCHID", wxBRUSHSTYLE_TRANSPARENT));
Good point, just uses wxTRANSPARENT_BRUSH now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26395)why use such strange API instead of adding
wxSVGFileGCderiving fromwxGraphicsContextas I'd naively expect? Is it significantly simpler to implement in this way?
My main motivation was to add graphics context rendering to an existing wxSVGFileGC, I hadn't thought of a stand alone wxSVGGraphicsContext handling everything by itself. Should I try to explore that?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 3 commits.
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + // Open a group with the given opacity. + void BeginLayer(double opacity); + + // Close the group opened by the most recent BeginLayer() call. + void EndLayer(); +
Done.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26395)why use such strange API instead of adding
wxSVGFileGCderiving fromwxGraphicsContextas I'd naively expect? Is it significantly simpler to implement in this way?My main motivation was to add graphics context rendering to an existing
wxSVGFileGC, I hadn't thought of a stand alonewxSVGGraphicsContexthandling everything by itself. Should I try to explore that?
I think it would be a more natural API from the user point of view, so if it's not too complicated to implement it like this, I think it would be preferable.
From the implementation point of view, I believe it might make sense to refactor the code to add some wxSVGFileWriter (or maybe just wxSVGWriter if we might generalize it to other kinds of output) in include/wx/private/svg.h and use it from both wxSVGFileDC and wxSVGFileGC.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 3b6f3ba Move helper functions into private header
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
> wxString GetSVGDocument() const;
bool Save();
private:
+
+ // String helpers used by both the DC and the SVG graphics context.
done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 5 commits.
- 956163f Abstract common SVG class internals into new wxSVGWriter
- 76ab81a Add create and CTOR methods to wxSVGGraphicsContext
- 33aa87a Document new methods
- 64b3d18 Remove unnecessary friendship
- c4c3813 Demonstrate new Create API in sample
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 608ec71 Remove private header dependancy from public header
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26395)@vadz Fulfills the expected API now and "plumbing" has been abstracted.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks, the API is much nicer now!
But I wonder if we could make it even more convenient by allowing to create wxSVGGC directly, without passing by wxSVGDC? In the sample we already have the latter anyhow, of course, but I'd expect application code to not need it when using wxSVGGC, so it looks like it could be useful to add a way to create it directly?
Also, I didn't look at the changes to the existing dcsvg.cpp carefully because it's difficult to do in the web UI, I'll do it later locally, but please let me know if there were any real changes that I should pay special attention to.
Thanks again!
> +#if wxUSE_GRAPHICS_CONTEXT +#include "wx/graphics.h" +#endif +
This could/should be replaced by wxGraphisContext forward declaration.
> @@ -265,6 +302,258 @@ bool MyPage::OnSave(const wxString& filename)
namespace
{
+#if wxUSE_GRAPHICS_CONTEXT
+// Shared content for the two SVGGraphicsContext demo pages: each page acquires
+// the GC differently, but draws the same shapes.
+void DrawSVGGraphicsContextDemo(wxGraphicsContext* gc, wxDC& dc)
Minor, but why do we pass both gc and dc here? And why is the former passed by pointer while the latter by reference? Seems inconsistent...
> + void AccumulatePathBounds(wxGraphicsPathData* data);
+
+ wxSVGFileDCImpl* m_impl;
+ wxSVGWriter* m_writer;
+ wxPen m_currentPen;
+ wxBrush m_currentBrush;
+ wxFont m_currentFont;
+ wxColour m_currentTextColour;
+
+ // Running current-transform-matrix plus save-state stack.
+ wxGraphicsMatrix m_transform;
+
+ struct State
+ {
+ wxGraphicsMatrix transform;
+ size_t clipNestingLevel;
I'd use int, this is not a size of anything.
> + wxSVGFileDCImpl* m_impl; + wxSVGWriter* m_writer;
Could these made const (as T* const, not const T*)? I think they should never change during the lifetime of the object.
> @@ -563,8 +234,104 @@ wxString wxSVGAttributes::GetAsString() const wxIMPLEMENT_ABSTRACT_CLASS(wxSVGFileDCImpl, wxDCImpl); -size_t wxSVGFileDCImpl::m_clipUniqueId = 0; -size_t wxSVGFileDCImpl::m_gradientUniqueId = 0; +size_t wxSVGWriter::ms_clipUniqueId = 0; +size_t wxSVGWriter::ms_gradientUniqueId = 0; + +// ---------------------------------------------------------- +// wxSVGFileDCImpl - string helpers (static)
Could this be kept above, to reduce the diff? I guess it would still show nicely with git --color-moved but not in GitHub web UI.
> + break;
+ }
+
+ case wxSVGPathSegment::ArcSegment:
+ {
+ wxDouble sweep = seg.endAngle - seg.startAngle;
+ constexpr wxDouble twoPi = 2.0 * M_PI;
+ if ( std::abs(sweep) >= twoPi - 1e-9 )
+ {
+ sweep = seg.clockwise ? twoPi : -twoPi;
+ }
+ else
+ {
+ if ( seg.clockwise )
+ {
+ while ( sweep <= 0 ) sweep += twoPi;
I hesitated to comment on this, but it's written on 2 lines below and it would be nice to split these loops on 2 lines too, both for consistency and readability.
> +wxGraphicsRenderer* wxSVGGraphicsRenderer::Get()
+{
+ static wxSVGGraphicsRenderer s_renderer;
+ return &s_renderer;
+}
+
I'm not sure how useful is this, what can be done with the renderer returned from here?
> +#include "wx/graphics.h"
+#include "wx/dcsvg.h"
+
+#include <stack>
+
+class wxSVGFileDCImpl;
+class WXDLLIMPEXP_FWD_CORE wxSVGWriter;
+
+// Singleton renderer producing wxSVGGraphicsContext and its supporting
+// handle/data objects. This renderer does not know how to create contexts
+// from arbitrary DCs or windows - the SVG graphics context is only ever
+// constructed indirectly via wxSVGFileDCImpl::GetGraphicsContext().
+class WXDLLIMPEXP_CORE wxSVGGraphicsRenderer : public wxGraphicsRenderer
+{
+public:
+ static wxGraphicsRenderer* Get();
Maybe this should take the path of the output file (and/or some wxSVGFile struct containing the path, size, DPI and title, and reused here and in wxSVGFileDC ctor)?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + void AccumulatePathBounds(wxGraphicsPathData* data);
+
+ wxSVGFileDCImpl* m_impl;
+ wxSVGWriter* m_writer;
+ wxPen m_currentPen;
+ wxBrush m_currentBrush;
+ wxFont m_currentFont;
+ wxColour m_currentTextColour;
+
+ // Running current-transform-matrix plus save-state stack.
+ wxGraphicsMatrix m_transform;
+
+ struct State
+ {
+ wxGraphicsMatrix transform;
+ size_t clipNestingLevel;
done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + void AccumulatePathBounds(wxGraphicsPathData* data);
+
+ wxSVGFileDCImpl* m_impl;
+ wxSVGWriter* m_writer;
+ wxPen m_currentPen;
+ wxBrush m_currentBrush;
+ wxFont m_currentFont;
+ wxColour m_currentTextColour;
+
+ // Running current-transform-matrix plus save-state stack.
+ wxGraphicsMatrix m_transform;
+
+ struct State
+ {
+ wxGraphicsMatrix transform;
+ size_t clipNestingLevel;
Same is true for the various depth levels. They were size_t before, but I can make them int for consistency.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
> + wxSVGFileDCImpl* m_impl; + wxSVGWriter* m_writer;
done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> +#if wxUSE_GRAPHICS_CONTEXT +#include "wx/graphics.h" +#endif +
done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26395)But I wonder if we could make it even more convenient by allowing to create
wxSVGGCdirectly, without passing bywxSVGDC? In the sample we already have the latter anyhow, of course, but I'd expect application code to not need it when usingwxSVGGC, so it looks like it could be useful to add a way to create it directly?
I was mimicking the current wxGraphicsContext API. If you are wanting a wxSVGGraphicsContext that can also run on its own without taking a wxSVGFileDC I can explore that, but this will take a new CTOR or Create method that has no analog in wxGraphicsContext. Are you thinking I should have a CTOR that takes the same params as wxSVGFileDC's CTOR?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> +wxGraphicsRenderer* wxSVGGraphicsRenderer::Get()
+{
+ static wxSVGGraphicsRenderer s_renderer;
+ return &s_renderer;
+}
+
Sorry, this comment was misguided/misplaced, please ignore it (not deleting it just to avoid confusion, but please consider it deleted).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> +#include "wx/graphics.h"
+#include "wx/dcsvg.h"
+
+#include <stack>
+
+class wxSVGFileDCImpl;
+class WXDLLIMPEXP_FWD_CORE wxSVGWriter;
+
+// Singleton renderer producing wxSVGGraphicsContext and its supporting
+// handle/data objects. This renderer does not know how to create contexts
+// from arbitrary DCs or windows - the SVG graphics context is only ever
+// constructed indirectly via wxSVGFileDCImpl::GetGraphicsContext().
+class WXDLLIMPEXP_CORE wxSVGGraphicsRenderer : public wxGraphicsRenderer
+{
+public:
+ static wxGraphicsRenderer* Get();
This is also wrong, sorry, I'll explain what I meant in another comment.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26395)But I wonder if we could make it even more convenient by allowing to create
wxSVGGCdirectly, without passing bywxSVGDC? In the sample we already have the latter anyhow, of course, but I'd expect application code to not need it when usingwxSVGGC, so it looks like it could be useful to add a way to create it directly?
I was mimicking the current
wxGraphicsContextAPI. If you are wanting awxSVGGraphicsContextthat can also run on its own without taking awxSVGFileDCI can explore that, but this will take a new CTOR or Create method that has no analog inwxGraphicsContext. I'll try adding a CTOR that takes the same params aswxSVGFileDC's CTOR, based on your comments.
This case doesn't fit nicely the wxGraphicsContext/wxGraphicsRenderer design, where the renderer is supposed to be able to create contexts for all the different DCs, which define the rendering target. In our case we have a single target, so I think it makes sense to have some
wxGraphicsContext* CreateContext(const wxSVGOutput&);
in wxSVGGraphicsRenderer which would create the context associated with the given output, and the corresponding
wxSVGGraphicsContext(const wxSVGOutput&);
in wxSVGGraphicsContext.
Right now wxSVGFileDC is used instead of the putative wxSVGOutput which kind of works, but is a bit weird: why should we need to create a wxDC-derived object just to create a wxGC? It would be better to just pass the same parameters as you pass when creating wxSVGFileDC, i.e.
const wxString& filename, int width = 320, int height = 240, double dpi = wxSVG_DEFAULT_DPI, const wxString& title = wxString()
directly to wxSVGGC ctor IMHO.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> @@ -265,6 +302,258 @@ bool MyPage::OnSave(const wxString& filename)
namespace
{
+#if wxUSE_GRAPHICS_CONTEXT
+// Shared content for the two SVGGraphicsContext demo pages: each page acquires
+// the GC differently, but draws the same shapes.
+void DrawSVGGraphicsContextDemo(wxGraphicsContext* gc, wxDC& dc)
Just takes a wxGraphicsContext now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + break;
+ }
+
+ case wxSVGPathSegment::ArcSegment:
+ {
+ wxDouble sweep = seg.endAngle - seg.startAngle;
+ constexpr wxDouble twoPi = 2.0 * M_PI;
+ if ( std::abs(sweep) >= twoPi - 1e-9 )
+ {
+ sweep = seg.clockwise ? twoPi : -twoPi;
+ }
+ else
+ {
+ if ( seg.clockwise )
+ {
+ while ( sweep <= 0 ) sweep += twoPi;
Done, fixed in a few places
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
> +wxGraphicsRenderer* wxSVGGraphicsRenderer::Get()
+{
+ static wxSVGGraphicsRenderer s_renderer;
+ return &s_renderer;
+}
+
Just used internally by other objects. I removed from docs and will be implemented "standalone" API separately.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26395)Right now
wxSVGFileDCis used instead of the putativewxSVGOutputwhich kind of works, but is a bit weird: why should we need to create awxDC-derived object just to create awxGC? It would be better to just pass the same parameters as you pass when creatingwxSVGFileDC, i.e.
const wxString& filename, int width = 320, int height = 240, double dpi = wxSVG_DEFAULT_DPI, const wxString& title = wxString()directly to
wxSVGGCctor IMHO.
Yes, working on that now. There will be a multifaceted API, where you can:
- create a standalone
wxSVGGraphicsContextwith the same CTORs thatwxSVGFileDChas, - create a
wxSVGGraphicsContextand connect your ownwxSVGFileDCinto it, - or create a
wxSVGFileDCand access thewxSVGGraphicsContextwithin it.
The latter two are for drop-in compatibility with the rest of the DC/GraphicsContext APIs. (This plugs write into my existing code, I don't have to make any changes to get fancy GC rendering in by SVG.)
The first one is a new way of doing things that is much simpler, as you have requested.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26395)Also, I didn't look at the changes to the existing
dcsvg.cppcarefully because it's difficult to do in the web UI, I'll do it later locally, but please let me know if there were any real changes that I should pay special attention to.
Not really, but a bit of moving code around. I abstracted a lot of the string building logic into the common writer class so that the DC and GC can share the same writing logic.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> @@ -563,8 +234,104 @@ wxString wxSVGAttributes::GetAsString() const wxIMPLEMENT_ABSTRACT_CLASS(wxSVGFileDCImpl, wxDCImpl); -size_t wxSVGFileDCImpl::m_clipUniqueId = 0; -size_t wxSVGFileDCImpl::m_gradientUniqueId = 0; +size_t wxSVGWriter::ms_clipUniqueId = 0; +size_t wxSVGWriter::ms_gradientUniqueId = 0; + +// ---------------------------------------------------------- +// wxSVGFileDCImpl - string helpers (static)
I've reorged a bit of content in here into the writer class. Sorry, I'm not sure where you want this to go.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 10 commits.
- c1b597c Use int, not size_t
- 9b4a63b Make other depth levels int, not size_t, and make pointer const
- 118a5fb Use forward declare instead of include
- c26567b Add explicit overloads
- cfdbeb1 Add direct creation support to wxSVGGraphicsContext
- 74c3cf4 Add saving, document access, and bitmap hander to wxSVGGraphicsContext
- fb853f3 Add new standalone wxSVGGC to sample
- 53e15a6 Fix formatting
- 42259e7 Document new standalone wxSVGGraphicsContext functions
- be8e937 Use new wxSVGGraphicsContext CTOR in sample
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Blake-Madden
Blake-Madden left a comment (wxWidgets/wxWidgets#26395)OK, now you can do something like:
wxSVGGraphicsContext gc("/users/joe/output.svg", 800, 700, 72, "My title");
gc.SetBitmapHandler(new wxSVGBitmapEmbedHandler());
// amazing drawing code
// ...
gc.Save();
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
Thanks, there is a lot of code here and I still couldn't really read all of it, but superficially everything looks good. It would be really great to have at least some unit tests for this, i.e. use the new class and verify that the output corresponds to the expected one — it would be obviously too difficult to cover all SVG features in a test suite, but at least some basic tests checking that this works as expected would be very nice.
The only other thing is that wxGraphics API, with its context/renderer split is not really appropriate for this kind of things, but I don't think there is anything we can do about it and have no choice but to provide all these dummy `CreateContext()` overloads always returning `nullptr` in the renderer. Let me cc @csomor just in case I'm missing some better way this could be done.
If there are no other comments, I'll merge it soon, with just the minor `explicit` additions below.
Thanks again!
> +
+/**
+ @class wxSVGGraphicsContext
+
+ wxSVGGraphicsContext is a specialized @ref wxGraphicsContext that allows drawing
+ to an SVG file using the @ref wxGraphicsContext API. This context provides
+ access to advanced vector features not available in the standard
+ @ref wxDC API, such as cubic and quadratic Bézier curves, gradient
+ brushes, and alpha blending.
+
+ This context is used when using @ref wxGraphicsContext on a @ref wxSVGFileDC.
+
+ Example of use:
+ @code
+ wxSVGFileDC dc("output.svg", 800, 600);
+ wxGraphicsContext* gc = dc.GetGraphicsContext();
Maybe the example should show creating `gc` directly from the output file name etc? I think it's more natural for a code without any compatibility constraints (i.e. I don't see why would you create `wxSVGFileDC` in the new code).
> +
+ @library{wxcore}
+ @category{gdi}
+ @since 3.3.3
+*/
+class wxSVGGraphicsContext : public wxGraphicsContext
+{
+public:
+ /**
+ Constructor.
+
+ The DC must outlive the context.
+
+ @since 3.3.3
+ */
+ wxSVGGraphicsContext(wxSVGFileDC& dc);
```suggestion
explicit wxSVGGraphicsContext(wxSVGFileDC& dc);
```
> + wxSVGGraphicsRenderer() = default;
+
+ wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxSVGGraphicsRenderer);
+};
+
+// wxGraphicsContext specialisation that writes SVG drawing commands into
+// the shared SVG buffer owned by wxSVGFileDCImpl. Constructed only via
+// wxSVGFileDCImpl::GetGraphicsContext(); state (pen/brush/font) is mirrored
+// back onto the owning DC so NewGraphicsIfNeeded() can keep the output
+// well-formed regardless of whether drawing came through the DC or the GC.
+class WXDLLIMPEXP_CORE wxSVGGraphicsContext : public wxGraphicsContext
+{
+public:
+ explicit wxSVGGraphicsContext(wxSVGFileDCImpl* impl);
+ explicit wxSVGGraphicsContext(wxSVGFileDC& dc);
+ wxSVGGraphicsContext(const wxString& filename,
```suggestion
explicit wxSVGGraphicsContext(const wxString& filename,
```
(this should have been done for `wxSVGFileDC` ctor(s) too, of course, but at least let us do it for the new class).
> +// wxGraphicsContext specialisation that writes SVG drawing commands into
+// the shared SVG buffer owned by wxSVGFileDCImpl. Constructed only via
+// wxSVGFileDCImpl::GetGraphicsContext(); state (pen/brush/font) is mirrored
+// back onto the owning DC so NewGraphicsIfNeeded() can keep the output
+// well-formed regardless of whether drawing came through the DC or the GC.
+class WXDLLIMPEXP_CORE wxSVGGraphicsContext : public wxGraphicsContext
+{
+public:
+ explicit wxSVGGraphicsContext(wxSVGFileDCImpl* impl);
+ explicit wxSVGGraphicsContext(wxSVGFileDC& dc);
+ wxSVGGraphicsContext(const wxString& filename,
+ int width = 320,
+ int height = 240,
+ double dpi = wxSVG_DEFAULT_DPI,
+ const wxString& title = wxString());
+ wxSVGGraphicsContext(const wxSize& size,
```suggestion
explicit wxSVGGraphicsContext(const wxSize& size,
```
> +public:
+ /**
+ Constructor.
+
+ The DC must outlive the context.
+
+ @since 3.3.3
+ */
+ wxSVGGraphicsContext(wxSVGFileDC& dc);
+
+ /**
+ Constructor for creating an SVG file directly.
+
+ @since 3.3.3
+ */
+ wxSVGGraphicsContext(const wxString& filename,
```suggestion
explicit wxSVGGraphicsContext(const wxString& filename,
```
> + Constructor for creating an SVG file directly.
+
+ @since 3.3.3
+ */
+ wxSVGGraphicsContext(const wxString& filename,
+ int width = 320,
+ int height = 240,
+ double dpi = wxSVG_DEFAULT_DPI,
+ const wxString& title = wxString());
+
+ /**
+ Constructor for creating an SVG file directly.
+
+ @since 3.3.3
+ */
+ wxSVGGraphicsContext(const wxSize& size,
```suggestion
explicit wxSVGGraphicsContext(const wxSize& size,
```
> + }
+ @endcode
+
+ @library{wxcore}
+ @category{gdi}
+ @since 3.3.3
+*/
+class wxSVGGraphicsContext : public wxGraphicsContext
+{
+public:
+ /**
+ Constructor.
+
+ The DC must outlive the context.
+
+ @since 3.3.3
There is no need to use `@since 3.3.3` for all member functions when the class itself has it IMO.
> + /** @name State stack */
+ ///@{
+ /**
+ Push the current state (like transformations, clipping region, and quality
+ settings) of the context on a stack.
+ Multiple balanced calls to PushState() and PopState() can be nested.
+ */
+ virtual void PushState() override;
+
+ /**
+ Pop the last state from the stack and restore the previous state
Blake-Madden
@Blake-Madden pushed 1 commit.
- b7cb15c Update include/wx/svggc.h
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 73be82c Update include/wx/svggc.h
—
View it on GitHub or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 274b306 Update interface/wx/svggc.h
—
View it on GitHub or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 1 commit.
- 9d5b718 Update interface/wx/svggc.h
—
View it on GitHub or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + + /** + Creates a native graphics sub-bitmap from an existing bitmap. + */ + virtual wxGraphicsBitmap CreateSubBitmap(const wxGraphicsBitmap& bitmap, + wxDouble x, wxDouble y, + wxDouble w, wxDouble h) override; + ///@} + + /** + Returns the name of the renderer, which is always "svg". + */ + virtual wxString GetName() const override; + + /** + Returns the version of the renderer.
The SVG specification that it targets. Updating the docs now...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Blake-Madden
> + /** @name State stack */
+ ///@{
+ /**
+ Push the current state (like transformations, clipping region, and quality
+ settings) of the context on a stack.
+ Multiple balanced calls to PushState() and PopState() can be nested.
+ */
+ virtual void PushState() override;
+
+ /**
+ Pop the last state from the stack and restore the previous state.
+ */
+ virtual void PopState() override;
+ ///@}
Removing redundant docs...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> + case wxSVGPathSegment::CurveSegment: + m->TransformPoint(&s.x1, &s.y1); + m->TransformPoint(&s.x2, &s.y2); + m->TransformPoint(&s.x, &s.y); + Extend(s.x1, s.y1); + Extend(s.x2, s.y2); + Extend(s.x, s.y); + break; + case wxSVGPathSegment::QuadCurveSegment: + m->TransformPoint(&s.x1, &s.y1); + m->TransformPoint(&s.x, &s.y); + Extend(s.x1, s.y1); + Extend(s.x, s.y); + break; + case wxSVGPathSegment::ArcSegment: + break;
I'll add a wxFAIL_MSG
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> +
+ // A bitmap must be selected before wrapping the memory DC in a wxGCDC
+ // (size is irrelevant).
+ wxBitmap metricBmp(1, 1);
+ wxMemoryDC memDc(metricBmp);
+ const double scale = memDc.GetPPI().y / SVG_DPI.y;
+ if ( scale != 1.0 && scale != 0.0 )
+ {
+ // SVG is DPI-independent, so we want text metrics at the default (96) DPI.
+ // Text does not scale linearly (at least on MSW), so scale the font size,
+ // rather than dividing the returned extent.
+ font.SetFractionalPointSize(font.GetFractionalPointSize() / scale);
+ }
+
+#if wxUSE_GRAPHICS_CONTEXT
+ // Measure through a wxGCDC so widths come from the platform's modern font engine
From my testing, this produces more accurate text sizing (e.g., labels would fit inside of measured rects correctly, while using wxDC I would have text going outside of them). It was fairly noticeable, especially at smaller scales.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden pushed 4 commits.
- 332ece0 Document what SVG renderer version means
- ca9ae37 Remove base class functions from docs
- bca2394 Add fail message in unreachable case
- 75ea7fc Simplify example in docs
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Blake-Madden
@Blake-Madden commented on this pull request.
> +
+/**
+ @class wxSVGGraphicsContext
+
+ wxSVGGraphicsContext is a specialized @ref wxGraphicsContext that allows drawing
+ to an SVG file using the @ref wxGraphicsContext API. This context provides
+ access to advanced vector features not available in the standard
+ @ref wxDC API, such as cubic and quadratic Bézier curves, gradient
+ brushes, and alpha blending.
+
+ This context is used when using @ref wxGraphicsContext on a @ref wxSVGFileDC.
+
+ Example of use:
+ @code
+ wxSVGFileDC dc("output.svg", 800, 600);
+ wxGraphicsContext* gc = dc.GetGraphicsContext();
done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26395)Thanks for the latest fixes! I've squashed all commits with some minor changes (moving things around to minimize the diff, restoring the probably inadvertently deleted comments and reintroducing write() helper to minimize the diff even more) and will push them soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.