New (v0.5.4) Canvas API: why restrict context managers to one __enter__ only?

[Juan-de-Salgado]

I have a small confusion with the new Canvas API. It's a great step in the right direction, but some corners don't make sense to me; example:

I'd like to create a state to hold some draw_image actions, so I can clear later that part of the canvas only. So:

draw_collection = canvas.state()
(assume I preserved the state as self.draw_collection on some object; I'm trying to be brief here.)

Then, repeatedly at several moments of the app's lifetime:

  with draw_collection:
    canvas.draw_image(...)

And eventually, at some later moment,

draw_collection.drawing_actions.clear()

in order to remove these draw_image actions but keep the rest of the canvas' content.

However, on the line "with draw_collection:" I get the exception
RuntimeError: A Canvas context manager can only be entered once, and only before any subsequent drawing actions are added.

My question is, what is the rationale for this limitation? It forces me to rewrite as

  draw_action = canvas.draw_image(...)
  draw_collection.drawing_actions.append(draw_action)

where the first statement presumably added draw_action to canvas.root_state.drawing_actions, so now I also need to remove it from there.

[Juan-de-Salgado]

Repro:
statecmtest.zip

Pressing "Add", then "Clear", works as intended.

A second press of "Add" raises an exception.

[HalfWhitt]

Hi there! Glad you're liking (some of) the changes. I've had my nose so close to the API that it's good to have the opportunity to talk to a real user trying it out. Sorry it took me so long to see this.

Conceptually, the idea is that the interface on the Canvas widget itself matches the HTML canvas API as closely as is feasible. It has optional Python extras, like the context managers, but they still map directly to "real" canvas commands being executed one after the other. HTML's canvas API is entirely linear. There's no way, in JavaScript, to go back and change or remove a prior action after the fact; you just have to draw it over again (potentially after clearing the area). So Canvas's methods and attributes work the same way — and if you do want to alter history, you can manipulate the drawing actions directly, as you've resorted to.

That said, I don't quite recall if there was any technical reason a context manager can't be opened more than once — the main issue was opening one out of order. In other words, things start breaking (behaving unintuitively) if we allow this:

fill = canvas.fill() # Adds a Fill object to the list of drawing actions.

# Call some more drawing methods here. These append more actions after the Fill.

with fill:
    # Now, any actions added here happen "inside" the Fill, *before* the above
    # instructions, e.g. something drawn here would get covered up by anything drawn
    # above.

But I don't recall if anything in particular breaks if we allowed this:

with canvas.fill() as fill:
    # Call some drawing methods

# Do some other stuff, but *not* drawing methods of the canvas.

with fill:
    # More drawing methods, which get appended to the same Fill object.

Conceptually, it's a little weird... the idea is supposed to be that the context manager is saving state when it enters, then filling and restoring state when it leaves. This looks like it should save/fill/restore a second time, but in reality you're retroactively inserting more actions before the fill-and-restore. Would you find this helpful?

Now, one more thing: the code you've shown doesn't quite do what you think it does. Or at least, it does, but it also does something else.

draw_action = canvas.draw_image(...)
draw_collection.drawing_actions.append(draw_action)

That first method draws the image to the canvas, in whatever state it currently is. Then you also add the resulting drawing action to the state in question, resulting in the same drawing action occuring twice within the canvas's total set of drawing actions. What you probably want to do is this:

from toga.widgets.canvas import DrawImage

draw_action = DrawImage(...)
draw_collection.drawing_actions.append(draw_action)

Things can get quite surprising if you start mixing the two "levels" of interface; calling the canvas's methods in between adding and removing your own drawing actions is a little akin to modifying a list while iterating over it. The methods work under the assumption that things are progressing linearly, so they might do things you don't expect if you change the ground under their feet.

[Juan-de-Salgado]

Thanks for the reply, and the additional notes!

While what I wanted is doable with list methods, as you suggest, I hope the context manager limitation gets relaxed at some point, as it is cleaner to write it that way.

Thanks again!

P.S.: personally I don't see any problem with the "out of order" added instructions; if you create a "fill", whatever you add under it will be drawn where the "fill" was inserted; this does not seem contradictory to me. You're adding to a tree, and things are painted as per the tree, as expected.

[HalfWhitt]

Actually, wait... now that I think about it more, it's actually less intuitive than that. If you enter multiple context managers out of order (assuming the error is removed), the canvas doesn't have any way of knowing in what order you entered them. As currently implemented, it can only ever append to the "latest" spot, in terms of the tree — for example, D in the following hypothetical:

root_state ─┬─ A
            ├─ B
            ├─ C ──┬─ E
            └─ D   └─ F

So in my example above, it wouldn't be appending inside the Fill. It's tracking the tip. Doing otherwise — always adding to the last one opened — would be possible, I imagine. But it would probably require keeping a monolithic stack of open states, especially since the same one could be added to multiple canvases. I'd prefer it if the states could remain self-contained, and not have to maintain a global stack.

The more I think about it, I wonder if, rather than altering how Canvas targets states, this might be an argument for un-deprecating states' drawing methods. Beforehand, this was the only way to put drawing actions inside them. This had a potential foot-gun:

ctx = canvas.context
ctx.do_stuff()
with ctx.ClosedPath() as closed:
    # What you need to do:
    closed.do_stuff()
    # What you could quite easily keep doing, but these operate outside
    # this subcontext, ignoring any translations, rotations, etc.:
    ctx.do_stuff()

In other words, the actual context manager wasn't really doing much; it only mattered what you called the methods on. The drawing methods were added to canvas in order to make them auto-track the latest open context manager. So that's the preferred "normal" / linear API... but maybe it would be worth keeping them (undeprecated) on the states as well, for retroactive editing. So in your code, you could do:

draw_collection.draw_image(...)

And that would always add to that state, no matter where or when you call it. This works right now, in fact — you'll just get a deprecation warning. (Keep in mind — any time you're adding from anywhere other than the canvas itself, remember to manually call canvas.redraw(). Right now we have a shim that lets the state drawing methods auto-draw, but that will probably be removed in the future, even if we reverse course on removing the methods.)

One wrinkle here is that the signatures of some of these methods changed in the revamp; that hasn't been too much of a problem, because we've been able to keep the old signature on the states, while implementing the new one on Canvas. If we undeprecated the state methods, we'd have to accommodate both signatures on states...

[HalfWhitt]

P.S.: personally I don't see any problem with the "out of order" added instructions; if you create a "fill", whatever you add under it will be drawn where the "fill" was inserted; this does not seem contradictory to me. You're adding to a tree, and things are painted as per the tree, as expected.

My argument here would be that anyone working with the "normal" API shouldn't have to know that; they can consider it an implementation detail, and instead just think about a series of linear commands. Now, if someone's storing references to drawing actions, they're already stepping outside of the intended usage for that — but I could also see how such a user might reasonably expect re-using that context manager to act like invoking those same parameters a second time (at the the present point). Preventing re-entry, in this case, is a guard rail against stepping outside of the timestream without meaning to (or even knowing it's possible).

[Juan-de-Salgado]

Thanks again for the details!

I have not followed the sources, but from the documentation, I was under the impression that the logic would be something like:

On canvas creation:
    canvas.current_state = canvas.root_state

On this_state __enter__:
    this_state.previous_state = canvas.current_state
    canvas.current_state = this_state

On this_state __exit__:
    canvas.current_state = this_state.previous_state

Thus no global stack was needed.

I tried the following code,

with canvas.fill(color="cyan"):
    canvas.rect(10, 10, 100, 100)

red_fill = canvas.fill(color="red")     # (1)

with red_fill:                          # (2)
    canvas.rect(30, 30, 100, 100)

with canvas.fill(color="darkcyan"):
    canvas.rect(50, 50, 100, 100)

print("In root_state:", [type(d).__name__ for d in canvas.root_state.drawing_actions])
print("In red_fill:", [type(d).__name__ for d in red_fill.drawing_actions])

and it outputs

In root_state: ['Fill', 'Fill', 'Fill']
In red_fill: ['Rect']

so clearly an action is being inserted inside red_fill. Unfortunately, I cannot test any difference from a "linear order", because adding any canvas action between (1) and (2) presently causes the exception "A Canvas context manager can only be entered once, and only before any subsequent drawing actions are added."

Thanks once more.

P.S.: I understand the need for the interface to be clear without recurring to implementation imaginations... but there must be some definition of what the Z-order should be; the grouping of actions into states will probably be very related to the Z-order definition.

[HalfWhitt]

By the way, tagging @corranwebster and @freakboy3742 here, in case they have any opinions or would like to follow along. They're the other two people most involved in the API revamp.

[freakboy3742]

I agree it's worth some experimentation; but I'm not sure if a "multiple entry context manager" is consistent with the intended design of the context manager.

My understanding is/was that the purpose of the context manager was to mirror the open/close behavior on files - the context manager "saves/opens" the state, and exiting the context manager "restores/closes" the state. To that end, it's not clear to me how a second "enter" should be interpreted - is it appending actions to the original state? Creating a new state savepoint?

If we follow the file analogy here, I guess the interpretation could be "a second open resets the state of the file" (which I guess would clear the context and starts from scratch, unless the "file" is "opened for append"...). I guess either of those interpretations could be incorporated without breaking backwards incompatibility; but I'm not sure if that would be more or less confusing.

However, based on the current state of things: Looking at the original example code provided, my read is that the approach being taken isn't what I'd expect. If the intention is that each press of the button adds another rectangle, it shouldn't be re-opening the context manager, but manipulating the context manager directly - something like:

    def _add_rect(self, *_):
        n = len(self.container.drawing_actions)

        with self.canvas.fill(color="#80ffff") as fill:
            n += 2
            self.canvas.rect(n * 5, n * 5, 100, 100)
        self.container.drawing_actions.append(fill)

The creation of the container context establishes where the state save occurs in the overall workflow; but inside that, the drawing actions of the container can be manipulated as desired.

Does that match your expectations, @HalfWhitt?

[HalfWhitt]

My understanding is/was that the purpose of the context manager was to mirror the open/close behavior on files - the context manager "saves/opens" the state, and exiting the context manager "restores/closes" the state. To that end, it's not clear to me how a second "enter" should be interpreted - is it appending actions to the original state? Creating a new state savepoint?

I think this depends on where you're coming to it from. If you're thinking about it in terms of the underlying tree of drawing actions, then it would be reasonable to expect it to simply append new actions to that state's list. We now have concrete evidence of a user doing so. : )

If, however, you're not worrying about any of that and just trying to use the HTML canvas API plus a couple of Python bells and whistles, you might reasonably expect it to create a new drawing action with the same parameters. Which it definitely won't do.

It would be nice to support the former, but the latter feels like a gotcha. And either is outside the scope of the intended use case.

    def _add_rect(self, *_):
        n = len(self.container.drawing_actions)

        with self.canvas.fill(color="#80ffff") as fill:
            n += 2
            self.canvas.rect(n * 5, n * 5, 100, 100)
        self.container.drawing_actions.append(fill)

Well... this wouldn't do quite what's intended either. self.canvas.fill(...) creates and inserts a fill; now the fill exists in two different spots within the canvas. What you'd actually want is this:

    def _add_rect(self, *_):
        n = len(self.container.drawing_actions)
        n += 2

        fill = Fill(fill_style="#80ffff")
        rect = Rect(n*5, n*5, 100, 100)
        fill.drawing_actions.append(rect)
        self.container.drawing_actions.append(fill)
        self.canvas.redraw()

(I seem to have uncovered a bug already, because Fill(color="#80ffff") should work. Oops.)

Or, if we were to decide to undeprecate the state methods, it would be a little simpler:

    def _add_rect(self, *_):
        n = len(self.container.drawing_actions)
        n += 2

        fill = self.container.fill(color="#80ffff")
        fill.rect(n*5, n*5, 100, 100)
        self.canvas.redraw()

I'm... kind of leaning toward that. I think allowing that syntax would probably lessen a perceived need to use the context managers for this.

[freakboy3742]

Undeprecating the state methods seems like a weird step to take having just gone to such lengths to deprecate them... but I also can't offer a good argument for why we shouldn't allow them when they're clearly useful in the sort of scenario you've described here.

In terms of "how do we explain this", it's essentially saying that a call to a drawing action can be added to any state - the best default state is the canvas itself, but you can call into any other state, and it will be added to the end of the drawing instructions for that state.

The headache it causes me is that context managers almost become redundant in this worldview. If I can sequentially add a fill to the canvas, and then add instructions to the fill... isn't that what a context manager was historically doing? Have we not just added 2 ways of doing the same thing?

[HalfWhitt]

Undeprecating the state methods seems like a weird step to take having just gone to such lengths to deprecate them... but I also can't offer a good argument for why we shouldn't allow them when they're clearly useful in the sort of scenario you've described here.

I'm not convinced we should, I'm just... entertaining the concept. There are still several valid benefits of the deprecation:

• Since we removed states' references to their canvas, they can't auto-redraw when modified (or won't be able to, sans shim). We can mention this in the deprecation warning... but only if the method issues one when called.
• For drawing methods whose signatures have changed, we don't have to support the new signature on states (or the old one on Canvas).
• It is One Less Way of Doing It. Speaking of which...

In terms of "how do we explain this", it's essentially saying that a call to a drawing action can be added to any state - the best default state is the canvas itself, but you can call into any other state, and it will be added to the end of the drawing instructions for that state.

I think what this discussion is recontextualizing for me is that it's not really the same thing. One is for standard linear use, the other is for directly modifying states. Although... now that I say it like that, maybe having the same methods in different places do similar but not quite the same thing is even worse.

I think I might've talked myself back out of this.

The headache it causes me is that context managers almost become redundant in this worldview. If I can sequentially add a fill to the canvas, and then add instructions to the fill... isn't that what a context manager was historically doing? Have we not just added 2 ways of doing the same thing?

If I understand you correctly, then yes, that's what the states' drawing methods did (and still do, while deprecated).

But in terms of what context managers were historically doing... they weren't managing any context. Nothing was different inside them vs. outside. In other words, these behaved identically:

with ctx.Stroke() as stroke:
    stroke.rect(...)

with ctx.Stroke() as stroke:
    pass
stroke.rect(...)

But the intent was to call each state's methods within its own context; and doing so was visually helpful, if mechanically irrelevant. The purpose of putting drawing methods (back) onto the canvas was to allow the context managers to auto-route the commands called while inside them.

And supposedly one benefit of removing the methods from states is to make a clearer delineation between this "auto-managed" mode vs. manually adding things to specific states. But apparently I need to make the docs much clearer on the distinction.

[Juan-de-Salgado]

Thanks to all participants on the discussion.

To me, it's not entirely a "context managers" issue. Conceptually, part of the issue is that "state" has 2 functions:
(1) a repository for drawing state, like a "brush"; and
(2) a collection of drawing actions.

It's the second part that feels a bit lacking. From the moment you expose and allow list methods (via the drawing_actions property), you are inviting the drawing order to be disrupted. (Not that I oppose to that disruption, on the contrary, it gives the developer control over the Z-order... just saying that concepts like a "linear order" or "appending actions" become now a bit blurry.)

At the risk of seemingly repeating the obvious with the above... I think it sets the stage for a discussion about context managers. Are context managers intended to serve function (1) only, and using them for (2) stresses the design?

And if context managers are intended to be so coupled with the lifetime of the state (as in the open/close analogy), what is the intended use of creating states "on their own", separate from context managers?

[HalfWhitt]

Conceptually, part of the issue is that "state" has 2 functions:
(1) a repository for drawing state, like a "brush"; and
(2) a collection of drawing actions.

I suppose that's a fair way of putting it. It represents a saving and restoring of state, with whatever actions are performed in between. It's intended to be a shortcut for (and direct translation of) a series of standard commands understandable by the HTML canvas. In other words, with canvas.state(): should act exactly like:

canvas.save()
# do the actions specified inside the context
canvas.restore()

with canvas.fill(): should be exactly like:

canvas.save()
# do the actions specified inside the context
canvas.fill()
canvas.restore()

...And so on.

Are context managers intended to serve function (1) only, and using them for (2) stresses the design? And if context managers are intended to be so coupled with the lifetime of the state, why allow states to be created aside from context managers?

I'm not entirely sure I'd describe the distinction that way. The context manager is handling the addition of drawing action to the collection... but it's an abstraction over it. Consider the following code:

with canvas.fill() as fill:
    # Do some stuff

with fill:
    # Do some other stuff

Following the above examples, it would be consistent to translate this to:

canvas.save()
# Do some stuff
canvas.fill()
canvas.restore()

canvas.save()
# Do some other stuff
canvas.fill()
canvas.restore()

Anyone who wants to implement that is braver than I am. On the other hand, if I understand you correctly, you'd prefer it to translate to this:

canvas.save()
# Do some stuff
# Do some other stuff
canvas.fill()
canvas.restore()

From a technical standpoint, that's at least more feasible. But it breaks the metaphor.

I would put it like this: using states as context managers is a convenience for streamlining use of the standard HTML-based API. That is the reason you can create states without using them as context managers (or at least the reason why you'd want to); the only real reason to do so is to manipulate them directly.

If this is too tedious for doing so:

fill = Fill(fill_style="#80ffff")
rect = Rect(n*5, n*5, 100, 100)
fill.drawing_actions.append(rect)
self.container.drawing_actions.append(fill)

Would this syntax help?

fill = self.container.fill(color="#80ffff")
fill.rect(n*5, n*5, 100, 100)

[Juan-de-Salgado]

Would this syntax help?

fill = self.container.fill(color="#80ffff")
fill.rect(n*5, n*5, 100, 100)

It's brief enough, so I'd say "yes". I imagine this is an argument for bringing back drawing methods on states.

But I'd kindly ask you to think of all these "lego" pieces from a conceptual, abstract perspective, beyond my little example; I'd hate to have these tailored to this particular discussion, and possibly restrict everyone else (or my future self). :)

Thanks again for the help and for your work.

P.S. Fuel-for-flames: personally I'd be more in favor of keeping a state associated with the canvas it was created from. Or even changing that association if a state is moved into another canvas or state.

[corranwebster]

To me, it's not entirely a "context managers" issue. Conceptually, part of the issue is that "state" has 2 functions:
(1) a repository for drawing state, like a "brush"; and
(2) a collection of drawing actions.

It's the second part that feels a bit lacking. From the moment you expose and allow list methods (via the drawing_actions property), you are inviting the drawing order to be disrupted. (Not that I oppose to that disruption, on the contrary, it gives the developer control over the Z-order... just saying that concepts like a "linear order" or "appending actions" become now a bit blurry.)

I can see what you're saying.

This may be less of an issue once #4163 is merged: that provides the equivalent of HTML Canvas' Path2D, which then gives an object which is pretty much the "collection of drawing actions" without any of the drawing state. With that available, there may be less of a need for the state to strongly support the "collection of drawing actions" use-case for states.

In terms of re-opening context managers to do more drawing, I think there is the potential for significant confusion about what happens in edge cases like:

red_fill = canvas.fill(color="red") 

with red_fill:
    canvas.rect(30, 30, 100, 100)

with canvas.stroke(color="darkcyan"):
    canvas.rect(50, 50, 100, 100)
    with red_fill:
        canvas.rect(70, 70, 100, 100)

Should the last rectangle draw with a dark cyan outline and a red fill or just the red fill? Does it draw on top of the second rectangle, or underneath it?

I think there might be an argument for a "style" type object which modifies state but doesn't hold any drawing actions and is multiply enterable as a context manager:

red_blue_style = canvas.style(fill_color='red', stroke_color='blue')
thick_line_style = canvas.style(line_width=5)

with red_blue_style:
    canvas.rect(30, 30, 100, 100)
    canvas.stroke()
    canvas.fill()

with thick_line_style:
    canvas.rect(50, 50, 100, 100)
    canvas.stroke()
    canvas.fill()

    with red_blue_style:
        canvas.rect(70, 70, 100, 100)
        canvas.stroke()
        canvas.fill()

I think the behaviour here is fairly unambiguous, because of the requirement to explicitly call the stroke/fill methods. But even then it's not entirely clear to me what would happen if these style objects were mutable and you changed them after drawing. It's also not clear whether it makes sense to have one "style" object, or separate "fill style" and "line style" objects.

In any case, I would lean towards keeping the current state objects as non-reenterable.

[HalfWhitt]

This may be less of an issue once #4163 is merged: that provides the equivalent of HTML Canvas' Path2D, which then gives an object which is pretty much the "collection of drawing actions" without any of the drawing state. With that available, there may be less of a need for the state to strongly support the "collection of drawing actions" use-case for states.

That's a good point.

I think there might be an argument for a "style" type object which modifies state but doesn't hold any drawing actions and is multiply enterable as a context manager:

red_blue_style = canvas.style(fill_color='red', stroke_color='blue')
thick_line_style = canvas.style(line_width=5)

with red_blue_style:
    canvas.rect(30, 30, 100, 100)
    canvas.stroke()
    canvas.fill()

with thick_line_style:
    canvas.rect(50, 50, 100, 100)
    canvas.stroke()
    canvas.fill()

    with red_blue_style:
        canvas.rect(70, 70, 100, 100)
        canvas.stroke()
        canvas.fill()

The intent here is clear, but I don't see how it would function. Where are the drawing actions all going, in this example? To the root context? I guess that could work if Style had a reference back to the canvas that created it, so it could insert a Save and attribute-setting actions (into the active state) when entered and then a Restore when exiting. But... at that point we've basically re-invented a more complex (or at least entirely different) version of State, with its planned additional keyword arguments. And it wouldn't persist as a DrawingAction, the way everything else does.

I think it might instead be worth popularizing a pattern something like this (once the parameters exist):

red_blue_style = {"fill_style": "red", "stroke_style": "blue"}
thick_line_style = {"line_width": 5}

with canvas.state(**red_blue_style):
    canvas.rect(30, 30, 100, 100)
    canvas.stroke()
    canvas.fill()

with canvas.state(**thick_line_style):
    canvas.rect(50, 50, 100, 100)
    canvas.stroke()
    canvas.fill()

    with canvas.state(**red_blue_style):
        canvas.rect(70, 70, 100, 100)
        canvas.stroke()
        canvas.fill()

[corranwebster]

The intent here is clear, but I don't see how it would function. Where are the drawing actions all going, in this example? To the root context? I guess that could work if Style had a reference back to the canvas that created it, so it could insert a Save and attribute-setting actions (into the active state) when entered and then a Restore when exiting. But... at that point we've basically re-invented a more complex (or at least entirely different) version of State, with its planned additional keyword arguments. And it wouldn't persist as a DrawingAction, the way everything else does.

I was assuming that each with statement implicitly created a new state with the modified style attributes. That said...

I think it might instead be worth popularizing a pattern something like this (once the parameters exist):

This is better. Particularly if Style is formalized as a TypedDict...

[Juan-de-Salgado]

Possible idea to play with:

with canvas.use(state):
    # actions

canvas.use(state) returns a context manager that holds the association between canvas and state, allowing stacking states upon nesting without the need for a state to store a canvas reference.

Different names needed for (a) a method as above, that just wraps an EXISTING state in a context manager, and (b) a method that CREATES a state and stores it in the context manager, to be returned by enter ("with canvas.whatever() as state:"). In both cases the context manager holds also a reference to the canvas.

[HalfWhitt]

I'm not entirely sure I follow your description... but in any case, I think I'd want to wait till the design and implementation of Path2D is ready (#4163), and see how well that works for the use-case of adding to collections of actions at aribtrary points in time, before adding a whole additional mechanism.

(Speaking of which, wow, I somehow missed the notification that PR was ready for review. Off I go to do some reading...)