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

pass subpixel dir to shader via glyph instances instead of text run #2207

Merged
merged 1 commit into from Dec 9, 2017

Conversation

@lsalzman
Copy link
Contributor

lsalzman commented Dec 9, 2017

This is a follow-up to #2198. I overlooked cs_text_run, which will also need to know about subpx_dir. Since we don't break up batches for these text shadow glyphs, and since subpixel direction now depends on the glyph format which is size/transform dependent, it just made a lot more sense to pass it in via the (now) unused user_data2 in the glyph instance. This also moves some decision logic out of the shader. Since all these parameters get routed out of prim.user_dataN almost straight to fetch_glyph, it makes some sense to put subpx_dir there anyway.


This change is Reviewable

@glennw
Copy link
Member

glennw commented Dec 9, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

📌 Commit 972bb5e has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

Testing commit 972bb5e with merge e967a9a...

bors-servo added a commit that referenced this pull request Dec 9, 2017
pass subpixel dir to shader via glyph instances instead of text run

This is a follow-up to #2198. I overlooked cs_text_run, which will also need to know about subpx_dir. Since we don't break up batches for these text shadow glyphs, and since subpixel direction now depends on the glyph format which is size/transform dependent, it just made a lot more sense to pass it in via the (now) unused user_data2 in the glyph instance. This also moves some decision logic out of the shader. Since all these parameters get routed out of prim.user_dataN almost straight to fetch_glyph, it makes some sense to put subpx_dir there anyway.

<!-- 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/2207)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

💔 Test failed - status-travis

@glennw
Copy link
Member

glennw commented Dec 9, 2017

@bors-servo retry

  • travis timeout
@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

Testing commit 972bb5e with merge 2c5c043...

bors-servo added a commit that referenced this pull request Dec 9, 2017
pass subpixel dir to shader via glyph instances instead of text run

This is a follow-up to #2198. I overlooked cs_text_run, which will also need to know about subpx_dir. Since we don't break up batches for these text shadow glyphs, and since subpixel direction now depends on the glyph format which is size/transform dependent, it just made a lot more sense to pass it in via the (now) unused user_data2 in the glyph instance. This also moves some decision logic out of the shader. Since all these parameters get routed out of prim.user_dataN almost straight to fetch_glyph, it makes some sense to put subpx_dir there anyway.

<!-- 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/2207)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: glennw
Pushing 2c5c043 to master...

@bors-servo bors-servo merged commit 972bb5e into servo:master Dec 9, 2017
2 of 3 checks passed
2 of 3 checks passed
Taskcluster (pull_request) TaskGroup: Pending (for pull_request.opened)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Dec 9, 2017
3 of 6 tasks complete
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

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