Summary: | Optimize Region for single rectangle case | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||||||||||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | cdumez, cmarcelo, commit-queue, ews-watchlist, kondapallykalyan, luiz, noam, simon.fraser, webkit-bug-importer, zalan | ||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
Attachments: |
|
Description
Antti Koivisto
2019-03-14 05:33:04 PDT
Created attachment 364656 [details]
patch
Created attachment 364659 [details]
patch
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. 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. (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. Created attachment 364783 [details]
patch
Created attachment 364785 [details]
patch
Created attachment 364789 [details]
patch
Created attachment 364790 [details]
patch
Created attachment 364791 [details]
patch
Created attachment 364793 [details]
patch
Created attachment 364794 [details]
patch
Created attachment 364795 [details]
patch
Comment on attachment 364795 [details] patch Clearing flags on attachment: 364795 Committed r242995: <https://trac.webkit.org/changeset/242995> All reviewed patches have been landed. Closing bug. |