Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSplit large unsafe block in rasterize_glyph() into multiple smaller ones #757
Conversation
|
Looks great, thanks for looking into this! I think that alignment issue should be fixed if it's not too much churn, what do you think? |
| let metrics = unsafe { &(*slot).metrics }; | ||
| let mut glyph_width = (metrics.width >> 6) as i32; | ||
| let glyph_height = (metrics.height >> 6) as i32; | ||
| let mut final_buffer = Vec::with_capacity(glyph_width as usize * |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hgallagher1993
Jan 19, 2017
Author
Contributor
Do mean have these lines lined up like this?
...(*slot).metrics
...(metrics.width >> 6)
...(metrics.height >> 6)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hgallagher1993
Jan 20, 2017
Author
Contributor
Ok I read over the code a load of times and I'm really not sure how I missed that. . .since it's only alignment will I just do --fixup?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hgallagher1993
Jan 20, 2017
Author
Contributor
That's this (from the servo workflow) git rebase -i --autosquash .....? . . .This is only my second pull request to an opensource project so still getting used to the work flow :-)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hgallagher1993
force-pushed the
hgallagher1993:local_branch
branch
from
8ebe3d9
to
be0a460
|
@bors-servo r+ |
|
|
Split large unsafe block in rasterize_glyph() into multiple smaller ones Fixes issue #515 r? @emilio not really sure who to assign to tbh <!-- 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/757) <!-- Reviewable:end -->
| let byte_value = if valid_byte { | ||
| let byte_index = (y * bitmap.pitch as i32) + (x >> 3); | ||
|
|
||
| unsafe { | ||
| let bit_index = x & 7; | ||
| let byte_ptr = bitmap.buffer.offset(byte_index as isize); | ||
| let bit = (*byte_ptr & (0x80 >> bit_index)) != 0; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hgallagher1993
Jan 20, 2017
Author
Contributor
Well bit_index definitely shouldn't be in there don't know why I left it in there tbh, but the line above *byte_ptr is unsafe as well let byte_ptr = bitmap.buffer.offset(byte_index as isize);
So it'd have to be changed to
let byte_ptr = unsafe { ... }
let bit = unsafe { ... }
No problem changing if you think it's worth it?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hgallagher1993
Jan 20, 2017
Author
Contributor
|
187 | let byte_ptr = bitmap.buffer.offset(byte_index as isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invocation of unsafe method
error: aborting due to previous error
error: Could not compile webrender.
Yup definitely!
|
|
hgallagher1993 commentedJan 19, 2017
•
edited
Fixes issue #515
r? @emilio not really sure who to assign to tbh
This change is