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

Use constant color blend for subpx text without clips. #1998

Merged
merged 1 commit into from Nov 7, 2017

Conversation

@glennw
Copy link
Member

glennw commented Nov 6, 2017

The current subpx rendering mode that supports clip masks is a
two pass technique. With the current batcher implementation,
this can result in a lot of batches on sites where text
bounding rects are a bit loose, and are detected as potentially
overlapping. This most often occurs on sites that use a lot
of small text.

As an interim solution, only use the clip mask supporting method
when a clip mask (or translucent text) is active. Otherwise, use
the old color blending method.

In the near(ish) future, we'll start using the dual source blending
extension for subpixel rendering on hardware that supports it. This
extension gives the best of both worlds - supporting clip masks and
being a single pass technique. It's supported on the vast majority
of our target hardware (perhaps even all of it).

On hardware that doesn't support dual source blending, using the
modes here which invoke a different technique depending on need
for alpha masking is a very reasonable tradeoff.

Additionally, future improvements to batching will improve this
fallback solution by being a bit smarter about clumping together
non-overlapping batches.

Fixes #1984.


This change is Reviewable

@glennw
Copy link
Member Author

glennw commented Nov 6, 2017

r? @kvark

Copy link
Member

kvark left a comment

LGTM, minus one small concern.
Plus it would make sense to start a try push for this, I think.

pub fn set_blend_mode_subpixel_opaque(&self, color: ColorF) {
self.gl.blend_color(color.r, color.g, color.b, color.a);
self.gl
.blend_func(gl::CONSTANT_COLOR, gl::ONE_MINUS_SRC_COLOR);

This comment has been minimized.

@kvark

kvark Nov 7, 2017

Member

we should be setting the blend equation here as well as in set_blend_mode_subpixel_passX

This comment has been minimized.

@glennw

glennw Nov 7, 2017

Author Member

Fixed

@mstange
Copy link
Contributor

mstange commented Nov 7, 2017

I think this method can be extended in a fairly straightforward way to cover non-opaque text as well (as long as the alpha is constant):

  • In the shader, use vec4(text.color.a) as the factor
  • Set the blend color to the unpremultiplied text color, i.e. color.r / color.a etc.

Then the resulting equation is

result.r = text_color.r / text_color.a * (text_color.a * mask.r) +
           (1 - text_color.a * mask.r) * dest.r
         = text_color.r * mask.r + (1 - text_color.a * mask.r) * dest.r
@mstange
Copy link
Contributor

mstange commented Nov 7, 2017

i.e. color.r / color.a etc.

Actually, if the color passed to set_blend_mode_subpixel_opaque is already unpremultiplied (which I think it is), then this would just be self.gl.blend_color(color.r, color.g, color.b, 1.0);.

The current subpx rendering mode that supports clip masks is a
two pass technique. With the current batcher implementation,
this can result in a lot of batches on sites where text
bounding rects are a bit loose, and are detected as potentially
overlapping. This most often occurs on sites that use a lot
of small text.

As an interim solution, only use the clip mask supporting method
when a clip mask (or translucent text) is active. Otherwise, use
the old color blending method.

In the near(ish) future, we'll start using the dual source blending
extension for subpixel rendering on hardware that supports it. This
extension gives the best of both worlds - supporting clip masks and
being a single pass technique. It's supported on the vast majority
of our target hardware (perhaps even all of it).

On hardware that doesn't support dual source blending, using the
modes here which invoke a different technique depending on need
for alpha masking is a very reasonable tradeoff.

Additionally, future improvements to batching will improve this
fallback solution by being a bit smarter about clumping together
non-overlapping batches.

Fixes #1984.
@glennw glennw force-pushed the glennw:subpx-work branch from 9e01903 to 8cd894d Nov 7, 2017
@glennw
Copy link
Member Author

glennw commented Nov 7, 2017

@mstange That's a good point - we can definitely apply that as a follow up optimization, and only use the 2 pass method when a clip mask is present.

@kvark Fixed up that comment. r=you if the try run I'm starting is green?

@kvark
kvark approved these changes Nov 7, 2017
Copy link
Member

kvark left a comment

thanks, :shipit:

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2017

📌 Commit 8cd894d has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2017

Testing commit 8cd894d with merge 71c1f3d...

bors-servo added a commit that referenced this pull request Nov 7, 2017
Use constant color blend for subpx text without clips.

The current subpx rendering mode that supports clip masks is a
two pass technique. With the current batcher implementation,
this can result in a lot of batches on sites where text
bounding rects are a bit loose, and are detected as potentially
overlapping. This most often occurs on sites that use a lot
of small text.

As an interim solution, only use the clip mask supporting method
when a clip mask (or translucent text) is active. Otherwise, use
the old color blending method.

In the near(ish) future, we'll start using the dual source blending
extension for subpixel rendering on hardware that supports it. This
extension gives the best of both worlds - supporting clip masks and
being a single pass technique. It's supported on the vast majority
of our target hardware (perhaps even all of it).

On hardware that doesn't support dual source blending, using the
modes here which invoke a different technique depending on need
for alpha masking is a very reasonable tradeoff.

Additionally, future improvements to batching will improve this
fallback solution by being a bit smarter about clumping together
non-overlapping batches.

Fixes #1984.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1998)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: kvark
Pushing 71c1f3d to master...

@bors-servo bors-servo merged commit 8cd894d into servo:master Nov 7, 2017
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.