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 upFix Windows UI and font squishing #16034
Conversation
highfive
commented
Mar 20, 2017
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @KiChjang (or someone else) soon. |
highfive
commented
Mar 20, 2017
|
|
Hrm, isn't that something a reftest could cover? Perhaps more problematic - are tests being run on Windows in CI? |
|
We do not run any tests on our windows builders yet. |
|
Yeah, I definitely think we could flesh out the testing a bit, but I'm of the mind of "let's get Windows stood up and in the nightlies... then when we have more eyes on it, we'll have people help us keep pushing quality forward" Seeing the higher order bit being just getting Windows out there. |
|
@jonathandturner I removed the mention of @codec-abc in the PR description because such mentions then trigger an email notification every time the merge commit is pushed to new GH repositories, please don't mention in there and only in comments instead. |
|
r? @metajack or someone who knows windows stuff |
|
@bors-servo r+ I was sad there was no documentation on struct FontMetrics in gfx/font.rs. Could you file this? |
|
|
Fix Windows UI and font squishing <!-- Please describe your changes on the following line: --> This PR: * updates the browserhtml dep (fixes #15255) * updates the Windows font metric calculation (fixes #15698, based on codec-abc #15937 (comment)) * may address #15933. With this PR, I was not able to repro --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because change are to UI/visual results of font drawing <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16034) <!-- Reviewable:end -->
|
|
|
@jonathandturner Thanks for knocking these out! |
windows: fix line height handling hi there! on windows, this change properly scales a font's line height by its size. before, this was not being done at all, which resulted in super ugly text at larger em sizes.  i'm not sure what to do about tests: i wasn't able to find any similar tests, and the previous change (#16034) did not include any tests either. i believe this also solves #16476. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17340) <!-- Reviewable:end -->
jonathandturner commentedMar 20, 2017
•
edited by nox
This PR:
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is