Optimize Region for single rectangle case

[Antti Koivisto]

Instrumentation shows vast majority of Region objects consist of a single rectangle. However it always allocates the large Shape data structure. This makes it unsuitable to use as a member in any popular objects.

[Antti Koivisto]

Created attachment 364656 [details]
patch

[Antti Koivisto]

Created attachment 364659 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 364659 [details]
patch

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

> Source/WebCore/platform/graphics/Region.cpp:60
> +Region::~Region()

= default

> Source/WebCore/platform/graphics/Region.cpp:85
> +        if (!m_bounds.isEmpty())
> +            rects.append(m_bounds);
> +        return rects;

Since this is the common case, maybe reserveInitialCapacity(1) and uncheckedAppend here, avoiding wasted vector capacity.

> Source/WebCore/platform/graphics/Region.h:189
> +    return a.m_bounds == b.m_bounds && arePointingToEqualData(a.m_shape, b.m_shape);

very nice

> Source/WebCore/platform/graphics/Region.h:226
> +    Optional<size_t> segmentIndex;
> +    decoder >> segmentIndex;

I don't really find this use of Optional<> and decoder >> much clearer than testing for decode success directly.

[Chris Dumez]

Comment on attachment 364659 [details]
patch

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

>> Source/WebCore/platform/graphics/Region.h:226
>> +    decoder >> segmentIndex;
> 
> I don't really find this use of Optional<> and decoder >> much clearer than testing for decode success directly.

It's not really about readability though. The reason we changed the way we decode was to not require a default constructor for the classes being decoded.

[Simon Fraser (smfr)]

(In reply to Chris Dumez from comment #4)

> It's not really about readability though. The reason we changed the way we
> decode was to not require a default constructor for the classes being
> decoded.

Makes sense.

[Antti Koivisto]

Created attachment 364783 [details]
patch

[Antti Koivisto]

Created attachment 364785 [details]
patch

[Antti Koivisto]

Created attachment 364789 [details]
patch

[Antti Koivisto]

Created attachment 364790 [details]
patch

[Antti Koivisto]

Created attachment 364791 [details]
patch

[Antti Koivisto]

Created attachment 364793 [details]
patch

[Antti Koivisto]

Created attachment 364794 [details]
patch

[Antti Koivisto]

Created attachment 364795 [details]
patch

[WebKit Commit Bot]

Comment on attachment 364795 [details]
patch

Clearing flags on attachment: 364795

Committed r242995: <https://trac.webkit.org/changeset/242995>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48925684>