Skip to content

Fix #347: Faster text rendering in Agg #5361

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 5, 2015
Merged

Fix #347: Faster text rendering in Agg #5361

merged 2 commits into from
Nov 5, 2015

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Oct 30, 2015

When the text isn't rotated, we can skip a lot of machinery by just doing a direct blending of text pixels onto the output buffer.

With the simple_plot_fps demo, I get about 6fps after, 5fps before this change.

Cc: @Tillsten: would you be willing to kick the tires on this one?

@tacaswell tacaswell added this to the proposed next point release (2.1) milestone Oct 30, 2015
@jkseppan
Copy link
Member

Note to reviewers: because of the indentation change, the github diff is big, but you can see the effective diff with

git remote add mdboom https://github.com/mdboom/matplotlib.git
git fetch mdboom
git show -w mdboom/fast-text

jkseppan
jkseppan reviewed Oct 31, 2015
View reviewed changes
src/_backend_agg.h
for (int xi = text.x1; xi < text.x2; ++xi) {
typename ImageArray::value_type pixel = image(
yi - (y - image.dim(0)), xi - x);
pixFmt.blend_pixel(xi, yi, gc.color, pixel);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not an agg expert, but it seems that blend_color_hspan could be even faster.

http://sourceforge.net/p/vector-agg/mailman/message/5116797/

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll play with that. blend_color_hspan doesn't take as inputs exactly the same thing that we have, so there may be overhead in creating that first negating any wins. Only benchmarking will tell.

@QuLogic
Copy link
Member

QuLogic commented Oct 31, 2015

Note to reviewers: because of the indentation change, the github diff is big

You can also add ?w=1 to the end of the URL to have the viewer ignore whitespace changes.

@mdboom
Copy link
Member Author

mdboom commented Nov 2, 2015

I've updated this to use blend_color_solid (which is the single color version of blend_color_hspan). It isn't measurably faster, but it's fewer lines of code for us.

mdboom added 2 commits November 2, 2015 12:21
@mdboom
Fix matplotlib#347: Faster text rendering in Agg
340c256 
When the text isn't rotated, we can skip a lot of machinery by just
doing a direct blending of text pixels onto the output buffer.
@mdboom
Use blend_color_solid as suggested by @jkseppan
83a1025
@mdboom mdboom modified the milestones: next major release (2.0), proposed next point release (2.1) Nov 3, 2015
@mdboom
Copy link
Member Author

mdboom commented Nov 3, 2015

I'm remilestoning to 2.0, since this will affect image comparison tests (for the better).

tacaswell added a commit that referenced this pull request Nov 5, 2015
@tacaswell
Merge pull request #5361 from mdboom/fast-text
c457f01 
Fix #347: Faster text rendering in Agg
@tacaswell tacaswell merged commit c457f01 into matplotlib:master Nov 5, 2015
tacaswell added a commit that referenced this pull request Nov 5, 2015
@tacaswell
Merge pull request #5361 from mdboom/fast-text
a340827 
Fix #347: Faster text rendering in Agg
@tacaswell
Copy link
Member

cherry picked to v2.0.x as a340827

@mdboom mdboom mentioned this pull request Nov 7, 2015
Closed
@mdboom mdboom deleted the fast-text branch November 10, 2015 02:46
@jenshnielsen jenshnielsen mentioned this pull request Jul 25, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@mdboom @jkseppan @QuLogic @tacaswell