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

Clean up draw_commands in areas potentially helpful for #2065 #2069

Conversation

pushfoo
Copy link
Member

@pushfoo pushfoo commented Apr 20, 2024

Changes

TL;DR: Try to improve readability ahead of camera refactor research & prototyping

Why?

TL;DR: Help @DragonMoffon & @DigiDuncan when working on cameras

  1. Good news: I finished investigating our pyglet 2.1 Vec* issues much sooner than expected
  • Looks like a bad merge + overzealous __eq__ check
  • Need to follow-up with pyglet team
  • Probably easily fixed with a pyglet==2.1dev3 release
  1. Rects would help make cameras less brittle (see Initial cleanup of arcade.types #2065's follow-up / description)
  • We have a repeated root cause of projection / bounding box bugs due to lack of nice rect abstractions
  • The bugs due to this keep piling up as I perform intensive code review (newest is that up vector breaks properties)
  1. Digi & Dragon's feasibility checks / "census" for rect abstraction may have glossed over function internals
    • Signatures inspected
    • Unsure about implementations, especially on hard to read code
  2. The comments here a requests for further refactoring
  3. Improving readability lowers friction for delivering those refactors in future PRs

Where?

TL;DR: Function bodies where things help draw rects or might use quads

In draw_commands.py, wherever things look like:

  1. Shared drawing helpers
  2. Directly rectangle related
  3. Appear to contain quad-like structures (see draw_polygon_outline)

How?

TL;DR: Make the code understandable before even attempting to fix how fast it runs

  1. Replace long ugly inlined logic with Color's:
    1. from_iterable classmethod
    2. normalized property
  2. Try to fail fast by putting these higher up:
    1. Window & context fetch
    2. Storing local refs to the context's shader & buffer singletons
    3. Color.from_iterable(color) validation
    4. Allocations of new array.arrays
  3. General readability improvements, including:
    1. Split nested one-liner expressions
    2. Store re-used quantities in local variables
    3. Add comments
    4. Remove redundant conversions such as the tuple in (array.array('B', tuple(generator))
    5. * unpacking positional point arguments instead of indexing

pushfoo added 9 commits April 20, 2024 07:25
* Move validation & unpacking ctx members to locals to top of functions

* Use Color's from_iterable + normalize

* Use named variables for goal buffer sizes to make buffer expansion clearer

* Add comments and attempt to make math more readable
* Copy to ctypes array earlier

* Skip tuple conversion for array.array since it takes generators
@pushfoo
Copy link
Member Author

pushfoo commented Apr 20, 2024

  1. I ran python -m arcade.examples.drawing_primitives and nothing seems off
  2. If needed, I run do pixel comparison between branch outputs later

@eruvanos eruvanos merged commit e1115ae into pythonarcade:development Apr 23, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

2 participants