Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circle.segments not recalculated when change radius #39

Open
mkraljt opened this issue May 4, 2020 · 4 comments
Open

Circle.segments not recalculated when change radius #39

mkraljt opened this issue May 4, 2020 · 4 comments

Comments

@mkraljt
Copy link

mkraljt commented May 4, 2020

I found this doing a quick porting of Tron demo from PyGameZero to entice my son to try making his own games.
When I animated the explosion circle it was a pentagon.

FYI:

  • Circle.segments and polygon isn't refreshed in _on_set_radius()
  • Circle segments constructor parameter isn't exposed by Layer.add_circle()
    • I'd looked to that as a workaround, if its undesirable to rebuild the opengl polygon when resizing a circle

Demo of problem:

import wasabi2d as w2d

scene = w2d.Scene(width=800, height=800)

circle_starts_small_filled = scene.layers[1].add_circle(
            pos=(200, 200),
            radius=2,
            color='CYAN',
            fill=True
        )
circle_starts_small = scene.layers[1].add_circle(
            pos=(200, 600),
            radius=2,
            color='CYAN',
            fill=False
        )
circle_starts_bigger_filled = scene.layers[1].add_circle(
            pos=(600, 200),
            radius=20,
            color='GREEN',
            fill=True
        )
circle_starts_bigger = scene.layers[1].add_circle(
            pos=(600, 600),
            radius=20,
            color='GREEN',
            fill=False
        )
w2d.animate(circle_starts_small, duration=5, radius=200)
w2d.animate(circle_starts_small_filled, duration=5, radius=200)
w2d.animate(circle_starts_bigger, duration=5, radius=200)
w2d.animate(circle_starts_bigger_filled, duration=5, radius=200)

# Found that segments is set on construction, so these get smoother:
w2d.animate( scene.layers[1].add_circle(
            pos=(200, 600),
            radius=5,
            color='CYAN',
            fill=False
        ), duration=5, radius=150)
w2d.animate( scene.layers[1].add_circle(
            pos=(200, 600),
            radius=10,
            color='CYAN',
            fill=False
        ), duration=5, radius=120)

w2d.run()
@lordmauve
Copy link
Owner

Yes, I hesitated about exposing segments in the factory method. It should be recalculated. But if the camera zooms, or the scale changes, it would be hard to guarantee a smooth circle. So I think I might be more inclined to expose it as something users can manage than to try to calculate it correctly always.

There is a workaround that works now: construct a large circle first, scale it down, then animate it scaling back to 1.

@mkraljt
Copy link
Author

mkraljt commented May 5, 2020

Yes I used the workaround of starting big and scaling down on the next line, before its drawn.

For a basic, tactical, fix maybe add_circle() could let user give an optional hint of a radius the circle should look good at, and explain this is a hint at the largest post scaling pixel radius. eg: radius_max_hint. A hint isn't great but might be worthwhile to document a user driven fix.

On the other hand, if in future multi-segment line allows changing the segments, then a circle could 'just work' by using that OpenGL segment rebuild logic.

@lordmauve
Copy link
Owner

It is relatively easy to implement, as vertex lists can already be resized. But it's expensive.

I think my original thinking was to expose segments in the constructor and make it updatable as an attribute. Only if you don't set it does it calculate an initial value.

@einarf
Copy link

einarf commented May 6, 2020

Just to throw in a random idea. Circles can be generated on the fly with geo shader fairly efficiently: https://github.com/pvcraven/arcade/tree/shader_experimental/arcade/resources/shaders/shapes/ellipse

Still there might be hardware out there being limited to 256 emitted vertices, but most have 4096. Limited the segments to 85 in this version, but that could be 340 if separated in 4 quadrants. It could be used to draw the circle on the fly or generate a buffer we draw later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants