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

Windows Servo renders reddit text squished #15698

Closed
jonathandturner opened this issue Feb 23, 2017 · 4 comments
Closed

Windows Servo renders reddit text squished #15698

jonathandturner opened this issue Feb 23, 2017 · 4 comments

Comments

@jonathandturner
Copy link

@jonathandturner jonathandturner commented Feb 23, 2017

As of #15681, the Windows build can now render reddit again (along with some other sites on the showcase that were failing). Yay!

There seems to be an issue rendering reddit with the current master, which displays reddit text like this:

reddit_with_patch

@larsbergstrom larsbergstrom mentioned this issue Feb 27, 2017
11 of 11 tasks complete
@codec-abc
Copy link

@codec-abc codec-abc commented Mar 13, 2017

I think I made some progress. The default font on Windows have a line gap of 0 which seems to be rather suspicious (at least to me). If I put this code

fn metrics(&self) -> FontMetrics {
    //....

    // This is the "fix"
    let fixed_line_gap = match dm.lineGap {
                0 => au_from_em(1 as f64),
                x => au_from_du(dm.lineGap as i32),
            };

    //..
    let metrics = FontMetrics {
        //...
        line_gap:         fixed_line_gap,
    };
    debug!("Font metrics (@{} pt): {:?}", self.em_size * 12., metrics);
    metrics
}

in components/gfx/platform/windows/font.rs (specifically the fn metrics(&self) -> FontMetrics function) I have this result
good_rendering

Since I have zero experience with Servo and Web technologies I leave up to you to judge if this the underlying problem and the appropriate fix.

PS : Sorry for the previous post.

@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Mar 13, 2017

@codec-abc - that looks a lot better. Want to send a PR? If there's a better fix, the reviewers might know, but at least it gets reddit working again.

@codec-abc
Copy link

@codec-abc codec-abc commented Mar 13, 2017

After more thinking I am quite sure that my changes fix the effect and not the cause. I think the issue is that the lineGap value should be read while parsing the font file but ended at 0 for some not yet understood reason. So maybe it is better to get the opinion of people that know this stuff better than me before sending a PR. (Note that I don't mind sending a PR, it just seem not necessary in this case).

@jonathandturner
Copy link
Author

@jonathandturner jonathandturner commented Mar 13, 2017

@codec-abc - right. that's why I was thinking a PR would help get the discussion going with a reviewer. It gives them something to look at and go "wait a sec, I think the actual issue is..."

@jonathandturner jonathandturner mentioned this issue Mar 20, 2017
4 of 5 tasks complete
bors-servo added a commit that referenced this issue Mar 21, 2017
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 -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 21, 2017
…ndturner:fix_font); r=metajack

<!-- Please describe your changes on the following line: -->

This PR:

* updates the browserhtml dep (fixes servo/servo#15255)
* updates the Windows font metric calculation (fixes servo/servo#15698, based on codec-abc servo/servo#15937 (comment))
* may address servo/servo#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. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: cea1760eb7689d88b18525fac7d53f9a12fdb8a1

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : e9654e401876e04082919212e16f185c2992cdc9
Manishearth pushed a commit to Manishearth/gecko-dev that referenced this issue Mar 28, 2017
…ndturner:fix_font); r=metajack

<!-- Please describe your changes on the following line: -->

This PR:

* updates the browserhtml dep (fixes servo/servo#15255)
* updates the Windows font metric calculation (fixes servo/servo#15698, based on codec-abc servo/servo#15937 (comment))
* may address servo/servo#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. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: cea1760eb7689d88b18525fac7d53f9a12fdb8a1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…ndturner:fix_font); r=metajack

<!-- Please describe your changes on the following line: -->

This PR:

* updates the browserhtml dep (fixes servo/servo#15255)
* updates the Windows font metric calculation (fixes servo/servo#15698, based on codec-abc servo/servo#15937 (comment))
* may address servo/servo#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. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: cea1760eb7689d88b18525fac7d53f9a12fdb8a1

UltraBlame original commit: fad072f940ccdfb70bb1799bb256841070e0993c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…ndturner:fix_font); r=metajack

<!-- Please describe your changes on the following line: -->

This PR:

* updates the browserhtml dep (fixes servo/servo#15255)
* updates the Windows font metric calculation (fixes servo/servo#15698, based on codec-abc servo/servo#15937 (comment))
* may address servo/servo#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. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: cea1760eb7689d88b18525fac7d53f9a12fdb8a1

UltraBlame original commit: fad072f940ccdfb70bb1799bb256841070e0993c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…ndturner:fix_font); r=metajack

<!-- Please describe your changes on the following line: -->

This PR:

* updates the browserhtml dep (fixes servo/servo#15255)
* updates the Windows font metric calculation (fixes servo/servo#15698, based on codec-abc servo/servo#15937 (comment))
* may address servo/servo#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. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: cea1760eb7689d88b18525fac7d53f9a12fdb8a1

UltraBlame original commit: fad072f940ccdfb70bb1799bb256841070e0993c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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