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

BUG : fix svg corner case #4388

Merged
merged 1 commit into from
Apr 30, 2015
Merged

Conversation

tacaswell
Copy link
Member

Under certain conditions the id of the clipPath node emitted by
the svg writer will be the same for multiple files. If both svg
fragments are embedded in the same document, then one of them is
removed, the clipping of the remaining fragment can become inconsistent.

See ipython/ipython#8133

Closes #4349

Under certain conditions the ``id`` of the `clipPath` node emitted by
the svg writer will be the same for multiple files.  If both svg
fragments are embedded in the same document, then one of them is
removed, the clipping of the remaining fragment can become inconsistent.

See ipython/ipython#8133

Closes matplotlib#4349
@tacaswell
Copy link
Member Author

There are probably less extreme methods of introducing this randomness, but this in guaranteed to work.

@ghost
Copy link

ghost commented Apr 28, 2015

Tested and works great.

@jenshnielsen
Copy link
Member

I'm 👍 on this

@WeatherGod
Copy link
Member

That is a great explanation for why adding randomness to the svg nodes shouldn't impact users.

@mdboom
Copy link
Member

mdboom commented Apr 28, 2015

Not so fast -- this means that if the same clipping path is used multiple times in the same figure, it will be stored multiple times. It's actually intended to be deterministic for this very reason. What we need instead, I think, is a random but document-specific hash. The simplest way to do this might be to generate the salt in the BackendSvg constructor and store it in a member variable, reusing it each time.

@tacaswell
Copy link
Member Author

@mdboom The key used for internal look up and the key used in the svg output are already different values, see https://github.com/tacaswell/matplotlib/blob/fix_svg_id/lib/matplotlib/backends/backend_svg.py#L449. The cache dictionary is keyed on an internally unique key (based on the cilp path details) and stores the tuple of the clip ptach + the oid (which is what is now randomized).

@mdboom
Copy link
Member

mdboom commented Apr 28, 2015

Ah -- sorry for missing that detail. Looks good to me, then.

@tacaswell
Copy link
Member Author

If everyone likes this, can someone merge it 😉

jenshnielsen added a commit that referenced this pull request Apr 30, 2015
@jenshnielsen jenshnielsen merged commit 2d91ca6 into matplotlib:master Apr 30, 2015
@tacaswell tacaswell deleted the fix_svg_id branch April 30, 2015 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SVG backend is assigning same id to clipPath elements
4 participants