Summary: | Canvas does not draw any text if the font is not fully loaded yet | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, bdakin, darin, d-r, eric, fmalita, haraken, hausmann, kenneth, mitz, ojan, pdr, schenney, tony, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
URL: | http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 87355 | ||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2012-11-27 04:23:23 PST
Created attachment 176232 [details]
Patch
Created attachment 176234 [details]
Patch
Fix typo in bug title.
Comment on attachment 176234 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176234&action=review > Source/WebCore/platform/graphics/Font.cpp:159 > +void Font::drawText(GraphicsContext* context, const TextRun& run, const FloatPoint& point, int from, int to, bool force) const isn't enum better than bool? > Source/WebCore/platform/graphics/Font.cpp:162 > + // except if the 'force' argument is set to true. so what will it draw if fonts are not yet loaded? Created attachment 176237 [details]
Patch
Converted test into a ref test as advised by Dominik offline.
Comment on attachment 176232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176232&action=review > LayoutTests/http/tests/canvas/canvas-slow-font-loading-expected.txt:3 > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". You actually see more > LayoutTests/http/tests/canvas/canvas-slow-font-loading-expected.txt:10 > +The 2 canvas should be identical (2 green 'X' on a red background). this should be moved up Comment on attachment 176237 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176237&action=review > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2306 > + maskImageContext->drawBidiText(font, textRun, FloatPoint(0, 0), true /*force*/); I don't think force is easy to understand here. Force what? (In reply to comment #5) > (From update of attachment 176232 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176232&action=review > > > LayoutTests/http/tests/canvas/canvas-slow-font-loading-expected.txt:3 > > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". > > You actually see more > > > LayoutTests/http/tests/canvas/canvas-slow-font-loading-expected.txt:10 > > +The 2 canvas should be identical (2 green 'X' on a red background). > > this should be moved up It's a ref test now. (In reply to comment #6) > (From update of attachment 176237 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176237&action=review > > > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2306 > > + maskImageContext->drawBidiText(font, textRun, FloatPoint(0, 0), true /*force*/); > > I don't think force is easy to understand here. Force what? Agreed. Mikhail's suggestion to use an enum instead is good. I plan to do that to make it more understandable. I'm currently think about a good enum name :) Created attachment 176255 [details]
Patch
Sadly, I'm getting a build error due to a conflict between "Complex" typedef from WTF::Complex and Complex enum value in Font.h. I'll reupload a patch renaming "Complex" enum value to "ComplexPath" to avoid conflict with WTF::Complex :( Created attachment 176262 [details]
Patch
Comment on attachment 176262 [details]
Patch
Based on Kenneth's offline feedback, I'll split the "Complex" enum value renaming to a separate (dependent) patch. Clearing flags for now.
(In reply to comment #10) > Sadly, I'm getting a build error due to a conflict between "Complex" typedef from WTF::Complex and Complex enum value in Font.h. > > I'll reupload a patch renaming "Complex" enum value to "ComplexPath" to avoid conflict with WTF::Complex :( My initial thinking was wrong. It is not conflicting with WTF::Complex. However, the "Complex" enum value in Font.h is conflicting with something on EFL port: I just don't know what yet. (In reply to comment #13) > (In reply to comment #10) > > Sadly, I'm getting a build error due to a conflict between "Complex" typedef from WTF::Complex and Complex enum value in Font.h. > > > > I'll reupload a patch renaming "Complex" enum value to "ComplexPath" to avoid conflict with WTF::Complex :( > > My initial thinking was wrong. It is not conflicting with WTF::Complex. However, the "Complex" enum value in Font.h is conflicting with something on EFL port: I just don't know what yet. Ah, I believe I finally found it: /usr/include/X11/X.h:#define Complex 0 /* paths may intersect */ Any proposal on how to fix this? Is renaming "Complex" to "ComplexPath" an acceptable solution for everyone? (In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #10) > > > Sadly, I'm getting a build error due to a conflict between "Complex" typedef from WTF::Complex and Complex enum value in Font.h. > > > > > > I'll reupload a patch renaming "Complex" enum value to "ComplexPath" to avoid conflict with WTF::Complex :( > > > > My initial thinking was wrong. It is not conflicting with WTF::Complex. However, the "Complex" enum value in Font.h is conflicting with something on EFL port: I just don't know what yet. > > Ah, I believe I finally found it: > /usr/include/X11/X.h:#define Complex 0 /* paths may intersect */ > > Any proposal on how to fix this? Is renaming "Complex" to "ComplexPath" an acceptable solution for everyone? Alternatively, I could add the following to Font.h: #ifdef Complex // "X11/X.h" defines Complex to 0. #undef Complex #endif Any preference? I would personally prefer the #undef as it requires less change. Created attachment 176303 [details]
Patch
Proposal with the #undef to fix EFL port build break and minimize the amount of changes.
Comment on attachment 176303 [details] Patch Clearing flags on attachment: 176303 Committed r135888: <http://trac.webkit.org/changeset/135888> All reviewed patches have been landed. Closing bug. The new test is failing on Mac. I filed https://bugs.webkit.org/show_bug.cgi?id=104954 |