-
Notifications
You must be signed in to change notification settings - Fork 28
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
Managing GraphicsState with a Stack is unsafe #68
Comments
Thanks for your discovery, David. Here is how I understand the issue: Given Graphics2D instance
Is this the problem you are describing? Also: Which UI Framework are you using (Swing, AWT)? |
Yes, I believe that captures the issue. Sorry, I should have written a simple minimal example like this for you. We're using Swing. I haven't managed to fix this so far, but I think the key is that as each Command is applied there needs to be a way to determine which GraphicsState should be active at the time. It could be through an instance-field association (direct or indirect) or through a look-up (like maybe a Map of States instead of a Stack). As your example shows, both 'g' and 'g2' are simultaneously "live" and so they need to work with independent state. |
I have an almost fully working fix for SVG and PDF but still need to extend it to EPS. It works fine in most cases but there are still issues with more complex UI screens. That could be related or might be a completely separate bug. I'll hold off on a PR until I understand that better. My original idea was basically OK, I just had things backward in dealing with GroupCommands. They need to be linked to the State of the last command in the Group; my original try used the first. I hope that makes sense. I'll describe things in more detail in the PR. |
Both SVGDocument and PDFDocument use a Stack to manage the current GraphicState instance while processing starting at a CreateCommand through the DisposeCommand. This is unsafe, though, because there's no guarantee the corresponding Graphics.create and Graphics.dispose calls will be made in nested pairs, or even that dispose will be called at all. Since this is a garbage-collected environment, this may happen behind the scenes through the finalize() method long after the SVG export.
This can lead to the wrong GraphicsState to be active during rendering. In our project, this is sometimes causing an issue with scrambled layouts: all the panels seem to be rendered properly but their layout in the document is incorrect.
The two SVG images in snapshots.zip show the issue. The 'orig' snapshot shows the scrambling, while 'fix' is a manual edit with the correct clip-path translations copied from a similar unscrambled snapshot.
I propose to attach the current GraphicsState to the StateCommand as an instance field instead, which can just be queried as needed during processing by SVGDoc and PDFDoc. I think this makes processing of CreateCommand and DisposeCommand into a no-op but haven't totally worked that out yet. It's possible there needs to be some transition-switch there.
I also haven't looked at the effect on EPS yet.
It could be kind of a big PR in terms of touching a large number of classes. The changes themselves ought to be pretty small, I think. I'm still working on it.
Hope this makes sense...
The text was updated successfully, but these errors were encountered: