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

Transform to resource transform when stroking pattern #6769

Closed
dsprenkels opened this issue Dec 17, 2015 · 6 comments
Closed

Transform to resource transform when stroking pattern #6769

dsprenkels opened this issue Dec 17, 2015 · 6 comments

Comments

@dsprenkels
Copy link
Contributor

Alternate title: Applying #6684-similar fix to stroke
#6684 makes sure the "current" transformation, at the moment of filling radial gradients is the baseTransform (that of the rescource (page), as it says in the specs). However, this should also be fixed in CanvasGraphics_stroke.

@timvandermeij
Copy link
Contributor

Are there testcases available that are currently broken because of this?

@dsprenkels
Copy link
Contributor Author

Currently; no. But it is not hard to write one.
PDF file (free to use): issue6769.pdf

Screenshots:

PDF.js (f93a220):
pdfjs

Reference (evince):
evince

@dsprenkels
Copy link
Contributor Author

Btw. I do not mind to fix this. But you may also label this as a good beginner bug, because a new developer can use the changes in #6684 as a lead for a fix.

@timvandermeij
Copy link
Contributor

Thank you for the update. If you are willing to fix this, feel free to make a pull request so we can review it.

@dsprenkels
Copy link
Contributor Author

I've been screwing around with this for some minutes. This is, because of the stroke width, a bit harder than expected. It will be necessary to fix the matrix transformation of the pattern itself, when dealing with pdf files like the one below.

I guess you can yet remove the 5-good-beginner-bug label; sorry.

issue6769.pdf
screenshot from 2015-12-17 15 58 15

@timvandermeij
Copy link
Contributor

Done. No worries, take as much time as you need, it's already great that you are willing to look into it! You can find us on IRC if you need any help with this.

brendandahl added a commit to brendandahl/pdf.js that referenced this issue May 11, 2021
Previously, we set the base transformation and pattern matrix
directly to the main rendering ctx of the page, however doing this
caused the current transform to be lost. This would cause issues
with things like shear missing so the pattern was misaligned or when
stroke was used the scale of the line width or dash would be wrong.
Instead we should leave the current transform and use setTransfrom
on the pattern so it is applied correctly. For axial and radial shadings I had
to create a temporary canvas to draw the shading so I could in turn
use setTransform.

Fixes: mozilla#13325, mozilla#6769, mozilla#7847, mozilla#11018, mozilla#11597

The following already in the corpus are improved:
issue8078-page1
issue1877-page1
brendandahl added a commit to brendandahl/pdf.js that referenced this issue May 11, 2021
Previously, we set the base transformation and pattern matrix
directly to the main rendering ctx of the page, however doing this
caused the current transform to be lost. This would cause issues
with things like shear missing so the pattern was misaligned or when
stroke was used the scale of the line width or dash would be wrong.
Instead we should leave the current transform and use setTransfrom
on the pattern so it is applied correctly. For axial and radial shadings I had
to create a temporary canvas to draw the shading so I could in turn
use setTransform.

Fixes: mozilla#13325, mozilla#6769, mozilla#7847, mozilla#11018, mozilla#11597, mozilla#11473

The following already in the corpus are improved:
issue8078-page1
issue1877-page1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants