As per the latest Canvas Element specification (http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html): "If a font is used before it is fully loaded, or if the font style source object does not have that font in scope at the time the font is to be used, then it must be treated as if it was an unknown font, falling back to another as described by the relevant CSS specifications." However, WebKit currently does not draw the text at all if the custom font is not fully loaded yet. We should handle this case better and follow the specification so that we use a fallback font for this case and draw the text.
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