Skip to content

BF: Save to the color cache when rendering.#6720

Merged
TEParsons merged 1 commit into
psychopy:devfrom
aforren1:color-stash
Jul 17, 2024
Merged

BF: Save to the color cache when rendering.#6720
TEParsons merged 1 commit into
psychopy:devfrom
aforren1:color-stash

Conversation

@aforren1
Copy link
Copy Markdown
Contributor

The _renderCache wasn't being used, and so the color was recomputed every call regardless of previous state. This improves some cases related to #6353, but not a total panacea. E.g.

from psychopy import visual
from timeit import default_timer
from statistics import mean, median

w = visual.Window([400, 400], checkTiming=False)
rct = []
for i in range(100):
    rct.append(visual.Rect(w, width=0.1, height=0.1, fillColor='red'))

times = []
for i in range(100):
    t0 = default_timer()
    for r in rct:
        r.draw()
    times.append(default_timer() - t0)
    w.flip()

w.close()

print(f'median: {median(times)}, mean: {mean(times)}')

Improves overall CPU time from ~17ms to ~4ms.

The _renderCache wasn't being used, and so the color was recomputed
every call regardless of previous state.
@peircej peircej requested a review from TEParsons July 17, 2024 08:58
Copy link
Copy Markdown
Contributor

@TEParsons TEParsons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Render cache is cleared when a new value is set so this seems all good. There's also plenty of tests in the test suite which check that changing a color value updates visually so I'm also fairly comfortable that any knock on effects from this would be caught.

@TEParsons TEParsons merged commit c00742b into psychopy:dev Jul 17, 2024
@peircej peircej added the 🐞 bug Issue describes a bug (crash or error) or undefined behavior. label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Issue describes a bug (crash or error) or undefined behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants