pgeu / pgeu-system

Mirror of the PostgreSQL Europe Conference and Membership management system.
MIT License
20 stars 28 forks source link

Support drawing circles in PDFs #159

Closed danielgustafsson closed 6 months ago

danielgustafsson commented 6 months ago

Does what it says on the tin, adds the ability to draw circles which can be handy when designing badges for example. Takes an optional fillcolor and strokecolor.

mhagander commented 6 months ago

Isn't changing the stroke color going to have an effect on future boxes? Since draw_box doesn't set the stroke color, isn't the one from a previous circle draw gong to override that one? And wold then require a similar handling in the draw_box to reset it?

danielgustafsson commented 6 months ago

Hmm .. that's right. AFAICT there is no way to ask reportLab for the current stroke color so it can't be saved off. That being said, if all routines that set a strokecolor always reset back to black then that's effectively the same as today right? The attached on top of the patch in the PR works fine even when drawing the circles before anything else.

diff --git a/postgresqleu/confreg/jinjapdf.py b/postgresqleu/confreg/jinjapdf.py
index 1288e1b5..e2fcb122 100755
--- a/postgresqleu/confreg/jinjapdf.py
+++ b/postgresqleu/confreg/jinjapdf.py
@@ -87,6 +87,7 @@ class JinjaFlowable(Flowable):
                          getmm(o, 'radius'),
                          stroke=stroke,
                          fill=fill)
+        self.canv.setStrokeColorRGB(0,0,0)

     def draw_box(self, o):
         if 'fill' in o:

So the contract is effectively: if the function sets stroke on/off the strokecolor will be black, if it sets a specific color it has to reset back to black before exiting.

Longer term I would like all shapes to have configurable stroke color so this is a nudge in that direction.

mhagander commented 6 months ago

I think the only two shapes that actually support stroke are box and circle, no? So based on that, I think we should see if we can fix it right away as part of this.

Basically, we'd have stroke=true (as defined now I think) or stroke=1 (which would work now) turn into stroke=black, and anything other than those map to get_color().

If that works without too much complexity, it seems to be better future-proof.

danielgustafsson commented 6 months ago

Ok, I've pushed a fix for this such that strokes can be set on both boxes and circles using the same syntax, along with docs. This conflates two things into one so if this goes in it will be split into two commits: "support for configurable stroke on box" and "support for circle".

mhagander commented 6 months ago

LGTM! Please go ahead.