Fix #6335: Queue boxes to update #6339

Merged
merged 3 commits into from May 2, 2016

Conversation

Projects
None yet
4 participants
Owner

mdboom commented Apr 26, 2016 edited

This fixes the issue where multiple animating axes would not all get updated. The reason this was broken is because blit sets a region to update, but there may be multiple blits between each opportunity to actually paint to the screen (on idle), so some will get thrown out. This changes things so there is a list (queue) of bounding boxes to update instead.

There is still the issue with glitchiness on resizing that I haven't been able to solve as easily.

Fixes #6335

@mdboom mdboom Queue boxes to update
37cb5e2

mdboom added the needs_review label Apr 26, 2016

Owner

tacaswell commented Apr 26, 2016

Probably on the resize event also nuke the blit quue?

Owner

mdboom commented Apr 26, 2016

Probably on the resize event also nuke the blit quue?

Strangely doesn't work.

Member

WeatherGod commented Apr 26, 2016

Would it make sense to utilize collections.deque here? Considering that blitting is for performance reasons, it might make sense to use it.

Owner

mdboom commented Apr 26, 2016

Would it make sense to utilize collections.deque here?

No, only matters if you care about the ordering of the popping, which we don't here.

Member

WeatherGod commented Apr 26, 2016

Strangely doesn't work.

Could it be due to the stale flag?

Owner

tacaswell commented Apr 26, 2016

unlikely, that only gets considered to decide if there should be a full re-draw.

mdboom added some commits Apr 26, 2016

@mdboom mdboom PEP8
84aeb5b
@mdboom mdboom Fix glitches on resize on Mac/Qt
5bde1ff
Owner

mdboom commented Apr 27, 2016

@dopplershift: The last commit here (5bde1ff) resolves the "glitches upon resize" issue (where the base parts of the figure outside of the blitted axes were not redrawn on resize). I have no idea if that's the right fix, but it should only decrease performance immediately after resizing -- it goes back to blitting only the minimal parts as normal right after that.

Member

WeatherGod commented Apr 27, 2016

just had a thought. I don't know if it is applicable here or not. It is technically possible to have an animation that exists across figures (essentially, they would all share the same timer). Is this list of bboxes guaranteed to be only from the one figure being rendered?

Owner

mdboom commented Apr 27, 2016

The list of bboxes is sorted with the canvas which is 1-1 with figures. So even if they shared a timer (a single global resource), each figure would have its own list of bboxes.

Owner

tacaswell commented Apr 28, 2016

attn @mfitzp @pwuertz @TD22057 as people who I think have worked extensively with qt + blitting.

Contributor

pwuertz commented May 1, 2016

I'm wondering if the blitboxes could be first combined into a bounding box in order to reduce the process to a single blit, but I guess there may be situations where this isn't necessarily a good idea. So without further profiling knowledge this looks good.

@tacaswell tacaswell merged commit 20c2961 into matplotlib:master May 2, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

tacaswell removed the needs_review label May 2, 2016

@tacaswell tacaswell added a commit that referenced this pull request May 2, 2016

@tacaswell tacaswell Merge pull request #6339 from mdboom/qt4agg-animation
Fix #6335: Queue boxes to update
f6f1f13
Owner

tacaswell commented May 2, 2016

backported to v2.x as f6f1f13

Could probably justify backporting this to v1.5.x as well, but hopefully we will not do a 1.5.2 so the point is moot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment