WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
238851
Unify the two ImageBuffer::create() functions, passing RenderingPurpose everywhere
https://bugs.webkit.org/show_bug.cgi?id=238851
Summary
Unify the two ImageBuffer::create() functions, passing RenderingPurpose every...
Simon Fraser (smfr)
Reported
2022-04-05 16:40:56 PDT
Unify the two ImageBuffer::create() functions, passing RenderingPurpose everywhere
Attachments
Patch
(54.06 KB, patch)
2022-04-05 16:47 PDT
,
Simon Fraser (smfr)
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(56.18 KB, patch)
2022-04-05 16:52 PDT
,
Simon Fraser (smfr)
sabouhallawa
: review+
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(55.63 KB, patch)
2022-04-05 20:50 PDT
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Patch
(54.09 KB, patch)
2022-04-05 21:01 PDT
,
Simon Fraser (smfr)
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2022-04-05 16:47:07 PDT
Created
attachment 456766
[details]
Patch
Simon Fraser (smfr)
Comment 2
2022-04-05 16:52:34 PDT
Created
attachment 456768
[details]
Patch
Said Abou-Hallawa
Comment 3
2022-04-05 17:24:56 PDT
Comment on
attachment 456768
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=456768&action=review
> Source/WebCore/platform/graphics/ImageBuffer.h:67 > + WEBCORE_EXPORT static RefPtr<ImageBuffer> create(const FloatSize&, OptionSet<ImageBufferOptions>, RenderingPurpose, float resolutionScale, const DestinationColorSpace&, PixelFormat, const CreationContext& = { });
I think ImageBufferOptions can be moved to the end just before CreationContext and it can have a default value { }. So many callers do not have to pass { } for it.
> Source/WebCore/platform/graphics/RenderingMode.h:47 > +enum class ImageBufferOptions : uint8_t {
Should this be moved to ImageBuffer.h since it is only used for ImageBuffer?
> Source/WebCore/platform/graphics/filters/software/FETileSoftwareApplier.cpp:54 > + OptionSet<ImageBufferOptions> bufferOptions; > + if (filter.renderingMode() == RenderingMode::Accelerated) > + bufferOptions.add(ImageBufferOptions::Accelerated);
This code is repeated many times in the patch. Can't we add an inline function in RenderingMode.h inline OptionSet<ImageBufferOptions> bufferOptions(RenderingMode renderringMode) { } Or something else. And use this inline directly in the call to ImageBuffer::create(tileRect.size(), bufferOptions(filter.renderingMode()), ...).
Simon Fraser (smfr)
Comment 4
2022-04-05 20:50:20 PDT
Created
attachment 456778
[details]
Patch
Simon Fraser (smfr)
Comment 5
2022-04-05 21:01:12 PDT
Created
attachment 456779
[details]
Patch
EWS
Comment 6
2022-04-06 08:23:50 PDT
Committed
r292469
(
249320@main
): <
https://commits.webkit.org/249320@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 456779
[details]
.
Radar WebKit Bug Importer
Comment 7
2022-04-06 08:24:17 PDT
<
rdar://problem/91354760
>
Darin Adler
Comment 8
2022-04-08 14:16:42 PDT
There’s a "RendingMode" typo in here. I suggest grepping for "endingMode".
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug