Bug 215410

Created attachment 406455 [details] Patch

Created attachment 406481 [details] Patch

<rdar://problem/67398052>

Created attachment 407560 [details] Patch

Created attachment 407618 [details] Patch

Comment on attachment 407618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407618&action=review > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:514 > - // Compute box final geometry. > + // Compute final box geometry. Solid progression. > Source/WebCore/layout/inlineformatting/InlineLineBox.cpp:191 > + auto lineIsConsideredEmpty = !lineHasImaginaryStrut || isLineVisuallyEmpty == IsLineVisuallyEmpty::Yes ? InlineBox::IsConsideredEmpty::Yes : InlineBox::IsConsideredEmpty::No; For this sort of things bools would read better.

(In reply to Antti Koivisto from comment #6) > Comment on attachment 407618 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407618&action=review > > > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:514 > > - // Compute box final geometry. > > + // Compute final box geometry. > > Solid progression. > > > Source/WebCore/layout/inlineformatting/InlineLineBox.cpp:191 > > + auto lineIsConsideredEmpty = !lineHasImaginaryStrut || isLineVisuallyEmpty == IsLineVisuallyEmpty::Yes ? InlineBox::IsConsideredEmpty::Yes : InlineBox::IsConsideredEmpty::No; > > For this sort of things bools would read better. Yeah, agree. I'll look into this (in a separate patch).

Created attachment 407850 [details] Patch

Committed r266509: <https://trac.webkit.org/changeset/266509> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407850 [details].

(In reply to EWS from comment #9) > Committed r266509: <https://trac.webkit.org/changeset/266509> Seems like this broke css2.1/t0905-c5525-fltwidth-00-c-g.html on ios-wk2. EWS also indicated that failure on previous version of this patch. History: https://results.webkit.org/?suite=layout-tests&test=css2.1%2Ft0905-c5525-fltwidth-00-c-g.html e.g.: https://build.webkit.org/results/Apple%20iOS%2013%20Simulator%20Release%20WK2%20(Tests)/r266509%20(6539)/results.html - RenderText {#text} at (392,0) size 784x519 + RenderText {#text} at (392,0) size 785x519 - text run at (745,360) width 39: "this is" + text run at (745,360) width 40: "this is"

(In reply to Aakash Jain from comment #10) > (In reply to EWS from comment #9) > > Committed r266509: <https://trac.webkit.org/changeset/266509> > Seems like this broke css2.1/t0905-c5525-fltwidth-00-c-g.html on ios-wk2. > EWS also indicated that failure on previous version of this patch. > > History: > https://results.webkit.org/?suite=layout-tests&test=css2.1%2Ft0905-c5525- > fltwidth-00-c-g.html > > e.g.: > https://build.webkit.org/results/ > Apple%20iOS%2013%20Simulator%20Release%20WK2%20(Tests)/r266509%20(6539)/ > results.html > > - RenderText {#text} at (392,0) size 784x519 > + RenderText {#text} at (392,0) size 785x519 > > - text run at (745,360) width 39: "this is" > + text run at (745,360) width 40: "this is" Thanks. looking into it now.

Thanks. This is somewhat urgent as it is slowing down iOS-wk2 EWS.

Actually the test was already marked as failing in r266565. So, not so urgent, but would be good to fix soon anyways.

(In reply to Aakash Jain from comment #13) > Actually the test was already marked as failing in r266565. So, not so > urgent, but would be good to fix soon anyways. Yeah I noticed it too. I briefly looked and it seemed like a rounding issue in the output and does not affect functionality. Will investigate.

(In reply to Aakash Jain from comment #13) > Actually the test was already marked as failing in r266565. So, not so > urgent, but would be good to fix soon anyways. Yeah, so as I suspected this is an float arithmetic issue: "This is a very unfortunate float arithmetic issue where the run's x position is now 0.000061px off -and the ceil() operation in "dump renderer as text" results in a different integral value -> rect.x(745.766968) rect.width(38.233074) ceilf 785.000000 rect.x(745.766907) rect.width(38.233074) ceilf 784.000000" will rebaseline.

Created attachment 408188 [details] Rebaseline patch

reopen for the rebaseline patch.

Committed r266707: <https://trac.webkit.org/changeset/266707> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408188 [details].