Bug 236923 - [GPU Process] [iOS] Sometimes the text drop shadow is not drawn
Summary: [GPU Process] [iOS] Sometimes the text drop shadow is not drawn
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Process Model (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks: 233914 236508
  Show dependency treegraph
 
Reported: 2022-02-19 21:59 PST by Jon Lee
Modified: 2022-05-13 15:13 PDT (History)
9 users (show)

See Also:


Attachments
Patch (7.52 KB, patch)
2022-04-01 20:17 PDT, Said Abou-Hallawa
simon.fraser: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (9.21 KB, patch)
2022-04-03 22:56 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2022-02-19 21:59:33 PST
fast/css/paint-order-shadow.html [ ImageOnlyFailure ]
imported/blink/svg/text/obb-paintserver.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/mathml/relations/css-styling/color-005.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-inline.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-writing-modes.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-variables/vars-font-shorthand-001.html [ ImageOnlyFailure ]
Comment 1 Radar WebKit Bug Importer 2022-02-19 21:59:50 PST
<rdar://problem/89196794>
Comment 2 Jon Lee 2022-03-14 23:32:34 PDT
obb-paintserver.html turns into a Crash, and paint-order-shadow.html still fails, but the others are resolved with the patch for 237728.
Comment 3 Said Abou-Hallawa 2022-03-15 11:32:33 PDT
The crash in orb-paintserver.html is fixed. It was happening because of infinite recursion. It was happening because of this new function 

inline bool operator==(const std::variant<Ref<Gradient>, Ref<Pattern>>& a, const std::variant<Ref<Gradient>, Ref<Pattern>>& b)
{
    return WTF::switchOn(a,
        [&] (const Ref<Gradient>& aGradient) {
            if (auto* bGradient = std::get_if<Ref<Gradient>>(&b))
                return aGradient == *bGradient;
            return false;
        },
        [&] (const Ref<Pattern>& aPattern) {
            if (auto* bPattern = std::get_if<Ref<Pattern>>(&b))
                return aPattern == *bPattern;
            return false;
        }
    );
}

The equality in "return aPattern == *bPattern;" was causing the recursion. Ref<Pattern> was converted to std::variant<Ref<Gradient>, Ref<Pattern>> so the function was calling itself infinitely. The fix is not to compare Ref<Pattern> but to compare the raw pointers: "return aPattern.ptr() == bPattern->ptr();"
Comment 4 Simon Fraser (smfr) 2022-03-15 12:34:41 PDT
Can we use arePointingToEqualData() here?
Comment 5 Jon Lee 2022-04-01 15:49:50 PDT
Remaining failures:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=fast%2Fcss%2Fpaint-order-shadow.html&test=imported%2Fblink%2Fsvg%2Ftext%2Fobb-paintserver.html

fast/css/paint-order-shadow.html [ ImageOnlyFailure ]
imported/blink/svg/text/obb-paintserver.html [ ImageOnlyFailure ]
Comment 6 Said Abou-Hallawa 2022-04-01 20:05:24 PDT
The failure:

imported/blink/svg/text/obb-paintserver.html [ ImageOnlyFailure ]

is tracked by bug 236924. Let this bug track the failure:

imported/blink/svg/text/obb-paintserver.html [ ImageOnlyFailure ]
Comment 7 Said Abou-Hallawa 2022-04-01 20:17:14 PDT
Created attachment 456432 [details]
Patch
Comment 8 Simon Fraser (smfr) 2022-04-01 20:49:07 PDT
Comment on attachment 456432 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456432&action=review

> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:51
>  Recorder::Recorder(const GraphicsContextState& state, const FloatRect& initialClip, const AffineTransform& initialCTM, DrawGlyphsRecorder::DeconstructDrawGlyphs deconstructDrawGlyphs)
>      : GraphicsContext(state)
> -    , m_drawGlyphsRecorder(*this, deconstructDrawGlyphs)
> +    , m_drawGlyphsRecorder(*this, initialCTM.xScale(), deconstructDrawGlyphs)

Does this do the right thing when zoomed?
Comment 9 Said Abou-Hallawa 2022-04-03 22:56:17 PDT
Created attachment 456531 [details]
Patch
Comment 10 Said Abou-Hallawa 2022-04-04 10:15:23 PDT
Comment on attachment 456432 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456432&action=review

>> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:51
>> +    , m_drawGlyphsRecorder(*this, initialCTM.xScale(), deconstructDrawGlyphs)
> 
> Does this do the right thing when zoomed?

yes
Comment 11 EWS 2022-04-04 10:22:00 PDT
Committed r292294 (249192@main): <https://commits.webkit.org/249192@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456531 [details].