Issue metadata
Sign in to add a comment
|
Likely regression in 2d canvas api
Reported by
ryanosw...@gmail.com,
Sep 28
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36 Steps to reproduce the problem: 1. Navigate to karmata.com (our website). Login is not required. What is the expected behavior? Text is properly centered vertically What went wrong? Text is no longer centered vertically. Did this work before? Yes 69.0.3497.100 Does this work in other browsers? N/A Chrome version: 71.0.3565.0 Channel: stable OS Version: 10.0 Flash Version: Most likely this is a change in behavior of fillText. Should be straightforward to isolate the change with a binary search, but I have not done this yet. I will add more information as I investigate.
,
Sep 30
,
Oct 1
Tried testing the issue with the URL provided in comment# 0 on chrome reported version# 71.0.3565.0 and on # 69.0.3497.100(as mentioned in comment# 0 "Did this work before?"). Compared the behavior of site loaded in chrome version# 71.0.3565.0 with #69.0.3497.100, seen same behavior on both the versions. @Reporter: Please find the attached screenshots of chrome version# 71.0.3565.0 with #69.0.3497.100 for your reference and provide your feedback on it. If possible could you please provide screenshots/screencast of the issue which help in better understanding and further triaging it. Thanks!
,
Oct 1
++ Tested the issue on Windows-10.
,
Oct 1
The issue IS present in your screenshots. All of the text is exactly 2 pixels higher in 71.0.3565.0.PNG
,
Oct 1
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 1
,
Oct 2
Testing team: given the submitter's feedback would you please run a per-revision bisect on this? Thank you.
,
Oct 2
Replying to comment #1. We recently fixed a long-lasting bug ( https://crbug.com/607053 ) where the textBaseline for top, center and bottom was not correct. According to the spec, setting textBaseline to 'center' should align the text's baseline to 50% of the EM square (defined by the text size). The previous implementation in chrome used to aligned to 50% of the measured text max ascent/descent which is slightly different (depending on the font) and can produce a small misalignment with previous versions of chrome. Firefox has done this correctly for a long time. See this small example: http://jsfiddle.net/davidqum/f5a71cen/ (The three M's, blue, red and black should align perfectly)
,
Oct 2
I'm not convinced that the "corrected" behavior is in fact correct. I suppose I will spend a few hours constructing a 'reduced test case' to illustrate the differences between chrome, firefox, and safari. In the meantime, I'd appreciate if you could re-open the bug with the -needs-feedback tag rather than the wontfix status.
,
Oct 2
Reopening with need-feedback. For a further discussion of what is the "correct" behavior you can also check this Firefox bug thread: https://bugzilla.mozilla.org/show_bug.cgi?id=737852
,
Oct 3
Attached is a comparison of 15px roboto (from google fonts) and 15px arial positioning with the exact same canvas calls on chrome, chrome canary, and firefox. For both fonts, production chrome more closely matches firefox's behavior. I imagine this warrants more testing over more fonts, etc, but for now it seems to me that the "fix" actually makes the situation considerably worse because the delta between firefox and chrome is now larger and the direction of the delta has changed (any apps that previously attempted to split the difference between chrome and firefox will now be off by ~2px). FWIW, we've experienced a lot of pain with cross-browser/device positioning of drawText calls. In cases where we need precise positioning, we've been forced to calibrate our draws on page load by sampling the draw position for each font. If this issue can't be resolved, we can simply enable 'calibration' for all fonts, but other users may suffer.
,
Oct 3
reupload of arial test case
,
Oct 3
,
Oct 3
Thank you for the test example. We were able to reproduce and found a rounding error that affected only textBaseline middle. The original change is still valid, but for certain fonts at certain sizes we were off by some pixels in the middle case. A fix should be coming soon.
,
Oct 4
,
Oct 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/339739f679a9ad4943d77add01513b409f60f416 commit 339739f679a9ad4943d77add01513b409f60f416 Author: David Quiroz Marin <davidqu@chromium.org> Date: Thu Oct 11 14:40:48 2018 Fix precision errors in canvas textBaseline For a long time when setting a textBaseline on canvas we have been rounding the returning values to always* use integers. This extra round operation seems to be unnecessary now and it is actually creating other alignment issues. This change fixes does alignment problems with no noticeable regressions. *For super tiny fonts (< 5px) we already returned floats. ( crbug.com/338908 ) Bug: 890515 Change-Id: I099af9834aaf28bb2770e080bdb849eb729c18fd Reviewed-on: https://chromium-review.googlesource.com/c/1260210 Reviewed-by: Fernando Serboncini <fserb@chromium.org> Commit-Queue: David Quiroz Marin <davidqu@chromium.org> Cr-Commit-Position: refs/heads/master@{#598758} [modify] https://crrev.com/339739f679a9ad4943d77add01513b409f60f416/third_party/blink/renderer/core/html/canvas/text_metrics.cc
,
Oct 11
I've verified that chromium after 598758 is consistent with firefox. Can you clarify whether any released versions of chromium will contain the fix for https://crbug.com/60705 but NOT contain the rounding fix?
,
Oct 11
You pointed to a very old crbug, which is probably not what you meant. Could you clarify which original fix are you worried about?
,
Oct 11
Sorry. I'm referring to the fix of https://bugs.chromium.org/p/chromium/issues/detail?id=607053 referenced by davidqu in comment 9. Prior to commit 598758, Chromium is actually worse than it was before the 'baseline' bug was fixed because two bugs cancel each other out. I am concerned that Chrome may release with the first fix (incorrect baseline) included but the second fix (rounding error) not yet included.
,
Oct 12
Tried reproducing the issue on chrome reported version# 71.0.3565.0 using Windows-10 with steps mentioned below: 1) Launched chrome reported version, dragged and dropped the 'test-arial.html' file provided in comment# 12 2) Compared the behavior with reported version with the build with fix, didn't observed any change in it @ryanoswald: Please find the attached screenshot for your reference and try to test this issue on latest chrome# 71.0.3578.0 and help us in verifying the fix. Thanks!
,
Oct 12
I just verified and both fixes landed on 71: Commit e5d4d080... initially landed in 71.0.3539.0 (#587855) Commit 339739f6... initially landed in 71.0.3578.0 (#598758) |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ryanosw...@gmail.com
, Sep 28