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

Hatch clipping #1898

Closed
wants to merge 1 commit into from
Closed

Conversation

pelson
Copy link
Member

@pelson pelson commented Apr 12, 2013

Fixes a genuine bug with hatch clipping in Agg & tidies up some of the _backend_agg.cpp code.

Note the change from typedef agg::scanline_p8 to typedef agg::scanline_u8 has had no noticeable detrimental impact (p==packed, u==unpacked, see http://www.antigrain.com/doc/scanlines/scanlines.agdoc.html#toc0001).

There are some changes in here which could fall into v1.2.x, but given our targets for v1.3 and the fact that I am actively working on _backend_agg.cpp, I think it is fine for it to go to master only.

@@ -226,6 +226,7 @@
'mathtext_asarray.py',
'patch_collection.py',
'path_patch_demo.py',
'pcolormesh_example.py',
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've been using this to measure performance of any changes I'm making (via the backend driver), so thought I would include it here.

@mdboom
Copy link
Member

mdboom commented Apr 12, 2013

Thanks. I just merged the Travis/inkscape fix into master, so I don't think you have it on your branch. Can you rebase on master so that Travis can run?

From the linked document, I can see how there are different performance implications of packed vs. unpacked, but how was that causing the hatched rendering bug? (Just for my own edification, really, nice to have a solution in any event).

All the cleanups etc. look good.

@pelson
Copy link
Member Author

pelson commented Apr 12, 2013

Note the change from typedef agg::scanline_p8 to typedef agg::scanline_u8 has had no noticeable detrimental impact (p==packed, u==unpacked, see http://www.antigrain.com/doc/scanlines/scanlines.agdoc.html#toc0001).

Sorry, somebody asked me a question as I was typing this... I meant to finish the sentence with "the change was not necessary to fix the hatching bug".

@mdboom
Copy link
Member

mdboom commented Apr 12, 2013

Ah -- so what is the purpose of the change from packed to unpacked? (Other than it being simpler...)?

@pelson
Copy link
Member Author

pelson commented Apr 12, 2013

Ah -- so what is the purpose of the change from packed to unpacked? (Other than it being simpler...)?

It is necessary to start looking at compound rasterization (which was what I started doing, before spotting this). Its all a little incestuous, but as far as I can tell, these changes also seem to make a minor performance increase.

@mdboom
Copy link
Member

mdboom commented Apr 12, 2013

Ok, no worries, then.

@pelson
Copy link
Member Author

pelson commented Apr 18, 2013

@mdboom - thoughts?

@mdboom
Copy link
Member

mdboom commented Apr 18, 2013

There are a bunch of seemingly legitimate test failures on Travis that all seem to have clip paths in common, so I think perhaps something related to clip paths is broken here. Unfortunately, I can't reproduce the problem locally when installing this branch, so it's anybody's guess at this point. (This is where getting the build products from Travis would be extremely useful, but unfortunately, that's not possible).

@pelson
Copy link
Member Author

pelson commented Apr 18, 2013

Ah, ok. I'll try to investigate on my own VM which emulates the travis environment and have a look into this.

@pelson pelson closed this May 14, 2013
@pelson pelson mentioned this pull request May 14, 2013
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.

None yet

2 participants