FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Upgrade agg to SVN version by mdboom · Pull Request #3777 · matplotlib/matplotlib · GitHub

Repository navigation

Upgrade agg to SVN version - #3777

Merged
tacaswell merged 9 commits into
matplotlib:masterfrom
mdboom:upgrade-agg
Nov 11, 2014
Merged

tacaswell merged 9 commits into
matplotlib:masterfrom
mdboom:upgrade-agg

Conversation

mdboom commented Nov 10, 2014

Copy link
Copy Markdown
Member

As a first step toward doing image interpolation in floating-point, and having (optional) floating-point rendering buffers for more accurate alpha blending, this upgrades our included copy of Agg to the current SVN version here: http://sourceforge.net/p/agg/svn/HEAD/tree/

This version of Agg is a fork of Agg 2.4, so it is still under a BSD license.

A few caveats: There is a regression in the alpha blending on RGBA 32-bit (8-bit-per-plane) buffers caused by doing the alpha blending calculation in 8-bits rather than 32-bits as it was before. I've communicated on the Agg mailing list about a solution, but I can't find the "right way" to do it. So, instead, I've included the one-off solution that uses the old calculation from Agg-2.4 verbatim.

The core Agg developers seem to see no need for releasing tagged versions of Agg. (Again, discussed on the mailing list). This may be a problem for some packagers, since matplotlib can no longer be built against the "system" Agg. Currently, Fedora uses a system Agg for its matplotlib package, but Debian, Arch, MacPorts, and Homebrew do not.

mdboom added this to the v1.5.x milestone Nov 10, 2014
mdboom self-assigned this Nov 10, 2014

mdboom commented Nov 10, 2014

Copy link
Copy Markdown
Member Author

The failing image comparisons are very subtle. Haven't got to the bottom of why yet, but the difference in colors is quite minor.

mdboom commented Nov 11, 2014

Copy link
Copy Markdown
Member Author

On close reading of the code, it seems that the new results are actually more correct. The image interpolation calculation is being done in 32-bit rather than 16-bit integers in the new Agg version resulting in slightly smaller rounding errors. I've updated the base images here.

Copy link
Copy Markdown
Member

I am seeing a bunch of extraneous whitespaces.

Copy link
Copy Markdown
Member

Are they coming from the SVN version of agg?

mdboom commented Nov 11, 2014

Copy link
Copy Markdown
Member Author

Yes. The Agg files are included verbatim.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

dropping a namespace? perhaps we should push up some patches to them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In this case, it's just the dropping of a redundant namespace specifier, since this code is in a namespace agg block. It's not actually changing the namespace in which rendering_buffer lives. Generally, agg doesn't using the namespace prefix everywhere in its internal code, which is typical C++ practice.

mdboom commented Nov 11, 2014

Copy link
Copy Markdown
Member Author

Should I trim the whitespace on Agg code now? It makes things easier now, but will make tracking Agg SVN harder in the long run, unless I can convince Agg to change their trailing whitespace policy, which seems unlikely.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

these are getting passed into functions uninitialized. That doesn't seem right to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actually, their addresses are passed to vertex, where they should be set, but they aren't currently, due to the fault of matplotlib-side code. See #3781.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

4 participants


Back | FazBrowse Home | New Git URL