.
Created attachment 430749 [details] Patch
Comment on attachment 430749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430749&action=review r=mews > Source/WebCore/page/FrameSnapshotting.h:57 > + OptionSet<SnapshotFlags> flags = { }; Nit - you don't need the `= { }` here. > Source/WebCore/page/FrameSnapshotting.h:59 > + std::optional<PixelFormat> pixelFormat = std::nullopt; > + std::optional<DestinationColorSpace> colorSpace = std::nullopt; Ditto (don't need the `= std::nullopt`).
Created attachment 430751 [details] Patch
Comment on attachment 430749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430749&action=review >> Source/WebCore/page/FrameSnapshotting.h:59 >> + std::optional<DestinationColorSpace> colorSpace = std::nullopt; > > Ditto (don't need the `= std::nullopt`). without these I seem to be getting errors related to missing field initializers :/
(In reply to Devin Rousso from comment #4) > Comment on attachment 430749 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430749&action=review > > >> Source/WebCore/page/FrameSnapshotting.h:59 > >> + std::optional<DestinationColorSpace> colorSpace = std::nullopt; > > > > Ditto (don't need the `= std::nullopt`). > > without these I seem to be getting errors related to missing field > initializers :/ I see. Maybe just remove the `=` then?
Created attachment 430755 [details] Patch
Committed r278565 (238563@main): <https://commits.webkit.org/238563@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430755 [details].
<rdar://problem/78956259>
Comment on attachment 430755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430755&action=review > Source/WebCore/page/FrameSnapshotting.h:59 > + std::optional<PixelFormat> pixelFormat { }; > + std::optional<DestinationColorSpace> colorSpace { }; We've been trying to move to making these kind of parameters non-optional, as you really always need to specify them. What was the rationale here for making them optional?
Comment on attachment 430755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430755&action=review >> Source/WebCore/page/FrameSnapshotting.h:59 >> + std::optional<DestinationColorSpace> colorSpace { }; > > We've been trying to move to making these kind of parameters non-optional, as you really always need to specify them. What was the rationale here for making them optional? Oh! I was not aware of that. I'll change these to be non-optional and specify them at callsites :) <https://webkit.org/b/226756> (Require that callsites of `SnapshotOptions` specify a `DestinationColorSpace` and `PixelFormat`)