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

Implement support for multiple text runs per shadow. #1469

Merged
merged 3 commits into from Jul 12, 2017

Conversation

@glennw
Copy link
Member

glennw commented Jul 12, 2017

This change is Reviewable

@glennw
Copy link
Member Author

glennw commented Jul 12, 2017

r? @kvark

…erface.

This allows multiple text runs to be combined in to one shadow, and fixes
a few existing bugs encountered along the way. It doesn't (yet) support
other items in the text shadow, but the structures are in place to make
this easy to add.
@glennw glennw force-pushed the glennw:push-shadow branch from a96afb5 to 02ca881 Jul 12, 2017
@Gankra
Copy link
Contributor

Gankra commented Jul 12, 2017

r+ on the api/wrench changes

@kvark
kvark approved these changes Jul 12, 2017
Copy link
Member

kvark left a comment

Looks reasonable to me!

PendingTextShadow {
shadow: shadow,
text_primitives: Vec::new(),
clip_and_scroll: clip_and_scroll,

This comment has been minimized.

@kvark

kvark Jul 12, 2017

Member

nit: could be clip_and_scroll,

shadow: text_shadow.shadow,
};

text_shadow.local_rect = text_shadow.local_rect

This comment has been minimized.

@kvark

kvark Jul 12, 2017

Member

no need to actually modify the local_rect here as opposed to just declaring a new variable (thus lifting mut on text_shadow)

if self.glyph_instances.is_empty() {
let src_glyphs = display_list.get(self.glyph_range);
for src in src_glyphs {
self.glyph_instances.push(GlyphInstance {

This comment has been minimized.

@kvark

kvark Jul 12, 2017

Member

nit: could look better with self.glyph_instances.extend(..)

// which will be blitted to the framebuffer.
let cache_width = (metadata.local_rect.size.width * device_pixel_ratio).ceil() as i32;
let cache_height = (metadata.local_rect.size.height * device_pixel_ratio).ceil() as i32;
let cache_size = DeviceIntSize::new(cache_width, cache_height);

This comment has been minimized.

@kvark

kvark Jul 12, 2017

Member

could probably write via (metadata.local_rect.size * device_pixel_ratio).ceil().to_i32()

@kvark
Copy link
Member

kvark commented Jul 12, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2017

📌 Commit 02ca881 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2017

Testing commit 02ca881 with merge e8e5082...

bors-servo added a commit that referenced this pull request Jul 12, 2017
Implement support for multiple text runs per shadow.

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

bors-servo commented Jul 12, 2017

☀️ Test successful - status-travis
Approved by: kvark
Pushing e8e5082 to master...

@bors-servo bors-servo merged commit 02ca881 into servo:master Jul 12, 2017
3 checks passed
3 checks passed
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.