Don't scale single-char text divs. #5221
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
This change makes scrolling noticeably smoother on files with many single-char text divs, such as the one in mozilla#1045. The trade-off is that the visual appearance of text selection in such documents is slightly worse, because more text divs overlap. This change also uses `scaleX(N)` instead of `scale(N, 1)`. This might be marginally more efficient in terms of JS string concatenation.
|
/botio-windows preview |
From: Bot.io (Windows)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/482b3ad87107556/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/482b3ad87107556/output.txt Total script time: 0.83 mins Published
|
yurydelendik
added a commit
that referenced
this pull request
Sep 8, 2014
…-divs Don't scale single-char text divs.
|
Thank you |
|
I just saw i forgot to comment on this issue. Now that yury merged it, i got reminded. Wouldn't it be possible to tweak the css, so that the overlapping sections do not look darker? |
|
If you give the text a solid selection color, but wrap all the text in a container element with "opacity", the selection will not get darker where it overlaps. |
|
i assume it also would lighten the load on the renderer |
CodingFabian
added a commit
to CodingFabian/pdf.js
that referenced
this pull request
Sep 9, 2014
As a consequence of merging mozilla#5221 it is more likely to have multiple overlapping selection divs inside the text layer. Because each individual element gets the selection style applied, the 30%opacity stacks, making a 60% bar visible where the overlap happens. As proposed by @rocallahan, this can be fixed by making the selection style solid and setting opacity for the overall layer. I assume also that this should make the work for the renderer easier, but was unable to bench it.
CodingFabian
added a commit
to CodingFabian/pdf.js
that referenced
this pull request
Sep 9, 2014
As a consequence of merging mozilla#5221 it is more likely to have multiple overlapping selection divs inside the text layer. Because each individual element gets the selection style applied, the 30%opacity stacks, making a 60% bar visible where the overlap happens. As proposed by @rocallahan, this can be fixed by making the selection style solid and setting opacity for the overall layer. I assume also that this should make the work for the renderer easier, but was unable to bench it.
CodingFabian
added a commit
to CodingFabian/pdf.js
that referenced
this pull request
Sep 9, 2014
As a consequence of merging mozilla#5221 it is more likely to have multiple overlapping selection divs inside the text layer. Because each individual element gets the selection style applied, the 30%opacity stacks, making a 60% bar visible where the overlap happens. As proposed by @rocallahan, this can be fixed by making the selection style solid and setting opacity for the overall layer. I assume also that this should make the work for the renderer easier, but was unable to bench it.
CodingFabian
added a commit
to CodingFabian/pdf.js
that referenced
this pull request
Sep 9, 2014
As a consequence of merging mozilla#5221 it is more likely to have multiple overlapping selection divs inside the text layer. Because each individual element gets the selection style applied, the 30%opacity stacks, making a 60% bar visible where the overlap happens. As proposed by @rocallahan, this can be fixed by making the selection style solid and setting opacity for the overall layer. I assume also that this should make the work for the renderer easier, but was unable to bench it.
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 change makes scrolling noticeably smoother on files with many
single-char text divs, such as the one in #1045. The trade-off is that
the visual appearance of text selection in such documents is slightly
worse, because more text divs overlap.
This change also uses
scaleX(N)instead ofscale(N, 1). This mightbe marginally more efficient in terms of JS string concatenation.
Here's a screenshot showing text selection and search selection prior to my change:
Here's the equivalent screenshot after my change:
The old code gave very little overlap, but some gaps. The new code gives more overlap, but no gaps. Neither of them look great!