Bug 198763 - [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates
Summary: [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on: 198736
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-11 12:54 PDT by Wenson Hsieh
Modified: 2019-06-24 21:53 PDT (History)
7 users (show)

See Also:


Attachments
WIP patch (11.74 KB, patch)
2019-06-12 08:56 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (17.32 KB, patch)
2019-06-24 15:34 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Less code, more words (19.41 KB, patch)
2019-06-24 17:21 PDT, Wenson Hsieh
simon.fraser: review+
Details | Formatted Diff | Diff
Patch for landing (23.16 KB, patch)
2019-06-24 20:24 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2019-06-11 12:54:33 PDT Comment hidden (obsolete)
Comment 1 Wenson Hsieh 2019-06-12 08:56:16 PDT Comment hidden (obsolete)
Comment 2 Wenson Hsieh 2019-06-24 14:22:16 PDT
<rdar://problem/51826266>
Comment 3 Wenson Hsieh 2019-06-24 15:34:12 PDT Comment hidden (obsolete)
Comment 4 Wenson Hsieh 2019-06-24 16:05:47 PDT
Comment on attachment 372807 [details]
Patch

I’m going to see if I can make this a bit less crazy.
Comment 5 Simon Fraser (smfr) 2019-06-24 16:18:39 PDT
Comment on attachment 372807 [details]
Patch

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

> Source/WebCore/rendering/style/RenderStyle.cpp:495
> +bool RenderStyle::isIdempotentTextAutosizingCandidate() const

You def. need some words about how this code came into being, and ideally you'd commit the tools used to generate this code, and the training dataset.
Comment 6 Wenson Hsieh 2019-06-24 16:32:05 PDT
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 372807 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=372807&action=review
> 
> > Source/WebCore/rendering/style/RenderStyle.cpp:495
> > +bool RenderStyle::isIdempotentTextAutosizingCandidate() const
> 
> You def. need some words about how this code came into being,

Yes, will do! I put some words in the overall WebCore ChangeLog entry, but this definitely warrants more detail.

> and ideally
> you'd commit the tools used to generate this code, and the training dataset.

Interesting...into open source, that is? I think we’ll need to check if that’s kosher :/
Comment 7 Simon Fraser (smfr) 2019-06-24 16:43:06 PDT
Maybe not, but to be maintainable, we have to be able to re-run your tools at any time.
Comment 8 Wenson Hsieh 2019-06-24 16:46:47 PDT
(In reply to Simon Fraser (smfr) from comment #7)
> Maybe not, but to be maintainable, we have to be able to re-run your tools
> at any time.

Indeed. They are already accessible to all of us (WebKit/Safari at Apple), but I’ll work towards getting the tools into the main Safari internal repository.
Comment 9 Wenson Hsieh 2019-06-24 17:21:39 PDT
Created attachment 372811 [details]
Less code, more words
Comment 10 Simon Fraser (smfr) 2019-06-24 17:33:01 PDT
Comment on attachment 372811 [details]
Less code, more words

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

> Source/WebCore/ChangeLog:17
> +        Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html

Be careful with "skip" in a test name (I think we have a convention that filenames with "skipped" in them are skipped). I would call this something else.

> Source/WebCore/rendering/style/RenderStyle.cpp:497
> +    auto fields = OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);

Need a comment at the top of this function referencing the radar that can point someone to the tools used to generate this code.
Comment 11 Wenson Hsieh 2019-06-24 20:24:18 PDT
Created attachment 372817 [details]
Patch for landing
Comment 12 WebKit Commit Bot 2019-06-24 21:06:58 PDT
Comment on attachment 372817 [details]
Patch for landing

Clearing flags on attachment: 372817

Committed r246781: <https://trac.webkit.org/changeset/246781>