SVG: optimize and refactor group creation code #7715
Merged
Conversation
This file contains 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
timvandermeij
force-pushed
the
svg-groups
branch
from
bf1cddd to
351bf25
Compare
yurydelendik
requested changes
Oct 15, 2016
| var url = queryParams.file || '../../test/pdfs/liveprogramming.pdf'; | ||
| var scale = +queryParams.scale || 1.5; | ||
| var DEFAULT_SCALE = 1.5; | ||
| var DEFAULT_URL = '../../web/compressed.tracemonkey-pldi-09.pdf'; |
There was a problem hiding this comment.
I would like to have flexibility to change file via query string during testing different files.
There was a problem hiding this comment.
Fixed in the new commits.
| this.pgrp.appendChild(this.defs); | ||
| this.pgrp.appendChild(this.tgrp); | ||
| this.svg.appendChild(this.pgrp); | ||
| this.svg.setAttributeNS(null, 'transform', pm(viewport.transform)); |
There was a problem hiding this comment.
this does not work for chrome -- we will need to create a root group and use it as this.svg ?
There was a problem hiding this comment.
Fixed in the new commits.
| /** | ||
| * @private | ||
| */ | ||
| _finishGroup: function SVGGraphics_finishGroup() { |
There was a problem hiding this comment.
_finishGroup can be removed, see yurydelendik@f4c8099
There was a problem hiding this comment.
Fixed in the new commits. Nice find!
This patch: - resolves a warning in the console about missing character encoding; - makes the viewer use the same background color and PDF file as the regular viewer; - simplifies the example to bring it more in line with the other examples.
timvandermeij
force-pushed
the
svg-groups
branch
from
351bf25 to
90797bf
Compare
This patch ensures that we only create transformation groups when it is actually required and that we re-use transform groups as much as possible. It reduces the number of transform groups for the Tracemonkey paper from 2790 to 1271, thereby making the DOM much lighter and rendering/scrolling smoother. Moreover, it simplifies the code and prevents duplication. Finally, we issue a warning when an unimplemented graphic state is encountered. Before, this was ignored silently, making debugging harder.
Each well-formed SVG image has the following structure: SVG element - Definitions element - Root group - Other group 1 - ... - Other group n This patch factors out initialization code into a private method in such as way that the creation of this structure is clear from the code. The root group is the replacement for the parent group from before. We need this group as we cannot apply the viewport transform on the SVG element itself (this caused issues in Chrome). If other code appends groups to the SVG image, in reality it is appending those groups to the root group, but this detail is abstracted away by this patch.
timvandermeij
force-pushed
the
svg-groups
branch
from
90797bf to
426fc45
Compare
timvandermeij
changed the title
|
I have updated the commits based on the review comments. Could you check again? |
yurydelendik
approved these changes
Oct 17, 2016
movsb
pushed a commit
to movsb/pdf.js
that referenced
this pull request
Jul 14, 2018
SVG: optimize and refactor group creation code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch series brings down the number of transform groups for the Tracemonkey paper from 2790 to 1271, resulting in a much lighter DOM and faster rendering/scrolling. Refer to the commit messages for more information.