Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move image metadata queries to a separate class named BitmapImageMetadata #23295

Closed
wants to merge 1 commit into from
Closed

Move image metadata queries to a separate class named BitmapImageMetadata #23295

wants to merge 1 commit into from

Conversation

shallawa
Copy link
Contributor

@shallawa shallawa commented Jan 26, 2024 •
edited by webkit-early-warning-system

5d5ba2e

Move image metadata queries to a separate class named BitmapImageMetadata
https://bugs.webkit.org/show_bug.cgi?id=268127
rdar://121643757

Reviewed by NOBODY (OOPS!).

ImageSource will delegate the image metadata queries to BitmapImageMetadata. The
metadata is retrieved either from the ImageDecoder for from the ImageFrame of the
primary index of the image.

* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::preTransformedNativeImageForCurrentFrame):
(WebCore::BitmapImage::draw):
(WebCore::BitmapImage::notSolidColor): Deleted.
(WebCore::BitmapImage::colorSpace): Deleted.
* Source/WebCore/platform/graphics/BitmapImage.h:
* Source/WebCore/platform/graphics/BitmapImageMetadata.cpp: Added.
(WebCore::BitmapImageMetadata::BitmapImageMetadata):
(WebCore::BitmapImageMetadata::imageMetadata const):
(WebCore::BitmapImageMetadata::primaryNativeImageMetadata const):
(WebCore::BitmapImageMetadata::primaryImageFrameMetadata const):
(WebCore::BitmapImageMetadata::encodedDataStatus const):
(WebCore::BitmapImageMetadata::size const):
(WebCore::BitmapImageMetadata::densityCorrectedSize const):
(WebCore::BitmapImageMetadata::orientation const):
(WebCore::BitmapImageMetadata::primaryFrameIndex const):
(WebCore::BitmapImageMetadata::frameCount const):
(WebCore::BitmapImageMetadata::repetitionCount const):
(WebCore::BitmapImageMetadata::colorSpace const):
(WebCore::BitmapImageMetadata::singlePixelSolidColor const):
(WebCore::BitmapImageMetadata::uti const):
(WebCore::BitmapImageMetadata::filenameExtension const):
(WebCore::BitmapImageMetadata::accessibilityDescription const):
(WebCore::BitmapImageMetadata::hotSpot const):
(WebCore::BitmapImageMetadata::maximumSubsamplingLevel const):
(WebCore::BitmapImageMetadata::dump const):
* Source/WebCore/platform/graphics/BitmapImageMetadata.h: Added.
(WebCore::BitmapImageMetadata::clear):
(WebCore::BitmapImageMetadata::source const):
* Source/WebCore/platform/graphics/Image.cpp:
(WebCore::Image::drawPattern):
(WebCore::Image::drawTiled):
* Source/WebCore/platform/graphics/Image.h:
(WebCore::Image::preTransformedNativeImageForCurrentFrame):
(WebCore::Image::singlePixelSolidColor const):
* Source/WebCore/platform/graphics/ImageFrame.cpp:
(WebCore::ImageFrame::size const): Deleted.
(WebCore::ImageFrame::singlePixelSolidColor const): Deleted.
* Source/WebCore/platform/graphics/ImageFrame.h:
(WebCore::ImageFrame::size const):
* Source/WebCore/platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::ImageSource):
(WebCore::ImageSource::dataChanged):
(WebCore::ImageSource::encodedDataStatusChanged):
(WebCore::ImageSource::encodedDataStatus const):
(WebCore::ImageSource::size const):
(WebCore::ImageSource::sourceSize const):
(WebCore::ImageSource::frameCount const):
(WebCore::ImageSource::notSolidColor const):
(WebCore::ImageSource::singlePixelSolidColor const):
(WebCore::ImageSource::clearMetadata): Deleted.
(WebCore::ImageSource::metadataCacheIfNeeded): Deleted.
(WebCore::ImageSource::firstFrameMetadataCacheIfNeeded): Deleted.
(WebCore::ImageSource::encodedDataStatus): Deleted.
(WebCore::ImageSource::frameCount): Deleted.
(WebCore::ImageSource::primaryFrameIndex): Deleted.
(WebCore::ImageSource::repetitionCount): Deleted.
(WebCore::ImageSource::uti): Deleted.
(WebCore::ImageSource::filenameExtension): Deleted.
(WebCore::ImageSource::accessibilityDescription): Deleted.
(WebCore::ImageSource::hotSpot): Deleted.
(WebCore::ImageSource::orientation): Deleted.
(WebCore::ImageSource::densityCorrectedSize): Deleted.
(WebCore::ImageSource::size): Deleted.
(WebCore::ImageSource::sourceSize): Deleted.
(WebCore::ImageSource::singlePixelSolidColor): Deleted.
(WebCore::ImageSource::maximumSubsamplingLevel): Deleted.
* Source/WebCore/platform/graphics/ImageSource.h:
* Source/WebCore/platform/graphics/NativeImage.h:
* Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp:
(WebCore::NativeImage::singlePixelSolidColor const):
* Source/WebCore/platform/graphics/cg/NativeImageCG.cpp:
(WebCore::NativeImage::singlePixelSolidColor const):

5d5ba2e

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@shallawa shallawa self-assigned this Jan 26, 2024
@shallawa shallawa added the Images For bugs in image handling. label Jan 26, 2024
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash c2532d9)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ⏳ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 gtk
⏳ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
⏳ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 api-gtk
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress
⏳ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa force-pushed the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch from c2532d9 to 1277112 Compare January 26, 2024 07:17
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 1277112)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac ❌ 🧪 api-wpe
⏳ 🧪 ios-wk2-wpt   🧪 mac-wk1 ❌ 🛠 gtk
⏳ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
⏳ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv-sim
⏳ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa force-pushed the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch from 1277112 to 5049602 Compare January 26, 2024 08:30
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 5049602)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac ❌ 🧪 api-wpe
⏳ 🧪 ios-wk2-wpt   🧪 mac-wk1 ❌ 🛠 gtk
⏳ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
⏳ 🛠 tv   🧪 mac-AS-debug-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv-sim
⏳ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa force-pushed the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch from 5049602 to a47de8d Compare January 26, 2024 09:45
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash a47de8d)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa force-pushed the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch from a47de8d to b6ed46a Compare January 26, 2024 15:10
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash b6ed46a)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ❌ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim ❌ 🧪 mac-wk2-stress
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 26, 2024
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label Jan 27, 2024
@shallawa shallawa force-pushed the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch from b6ed46a to fd814ac Compare January 27, 2024 01:00
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash fd814ac)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 27, 2024
@shallawa shallawa requested review from smfr and heycam and removed request for zdobersek and magomez January 28, 2024 01:27
@shallawa
Move image metadata queries to a separate class named BitmapImageMeta…
5d5ba2e 
…data

https://bugs.webkit.org/show_bug.cgi?id=268127
rdar://121643757

Reviewed by NOBODY (OOPS!).

ImageSource will delegate the image metadata queries to BitmapImageMetadata. The
metadata is retrieved either from the ImageDecoder for from the ImageFrame of the
primary index of the image.

* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::preTransformedNativeImageForCurrentFrame):
(WebCore::BitmapImage::draw):
(WebCore::BitmapImage::notSolidColor): Deleted.
(WebCore::BitmapImage::colorSpace): Deleted.
* Source/WebCore/platform/graphics/BitmapImage.h:
* Source/WebCore/platform/graphics/BitmapImageMetadata.cpp: Added.
(WebCore::BitmapImageMetadata::BitmapImageMetadata):
(WebCore::BitmapImageMetadata::imageMetadata const):
(WebCore::BitmapImageMetadata::primaryNativeImageMetadata const):
(WebCore::BitmapImageMetadata::primaryImageFrameMetadata const):
(WebCore::BitmapImageMetadata::encodedDataStatus const):
(WebCore::BitmapImageMetadata::size const):
(WebCore::BitmapImageMetadata::densityCorrectedSize const):
(WebCore::BitmapImageMetadata::orientation const):
(WebCore::BitmapImageMetadata::primaryFrameIndex const):
(WebCore::BitmapImageMetadata::frameCount const):
(WebCore::BitmapImageMetadata::repetitionCount const):
(WebCore::BitmapImageMetadata::colorSpace const):
(WebCore::BitmapImageMetadata::singlePixelSolidColor const):
(WebCore::BitmapImageMetadata::uti const):
(WebCore::BitmapImageMetadata::filenameExtension const):
(WebCore::BitmapImageMetadata::accessibilityDescription const):
(WebCore::BitmapImageMetadata::hotSpot const):
(WebCore::BitmapImageMetadata::maximumSubsamplingLevel const):
(WebCore::BitmapImageMetadata::dump const):
* Source/WebCore/platform/graphics/BitmapImageMetadata.h: Added.
(WebCore::BitmapImageMetadata::clear):
(WebCore::BitmapImageMetadata::source const):
* Source/WebCore/platform/graphics/Image.cpp:
(WebCore::Image::drawPattern):
(WebCore::Image::drawTiled):
* Source/WebCore/platform/graphics/Image.h:
(WebCore::Image::preTransformedNativeImageForCurrentFrame):
(WebCore::Image::singlePixelSolidColor const):
* Source/WebCore/platform/graphics/ImageFrame.cpp:
(WebCore::ImageFrame::size const): Deleted.
(WebCore::ImageFrame::singlePixelSolidColor const): Deleted.
* Source/WebCore/platform/graphics/ImageFrame.h:
(WebCore::ImageFrame::size const):
* Source/WebCore/platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::ImageSource):
(WebCore::ImageSource::dataChanged):
(WebCore::ImageSource::encodedDataStatusChanged):
(WebCore::ImageSource::encodedDataStatus const):
(WebCore::ImageSource::size const):
(WebCore::ImageSource::sourceSize const):
(WebCore::ImageSource::frameCount const):
(WebCore::ImageSource::notSolidColor const):
(WebCore::ImageSource::singlePixelSolidColor const):
(WebCore::ImageSource::clearMetadata): Deleted.
(WebCore::ImageSource::metadataCacheIfNeeded): Deleted.
(WebCore::ImageSource::firstFrameMetadataCacheIfNeeded): Deleted.
(WebCore::ImageSource::encodedDataStatus): Deleted.
(WebCore::ImageSource::frameCount): Deleted.
(WebCore::ImageSource::primaryFrameIndex): Deleted.
(WebCore::ImageSource::repetitionCount): Deleted.
(WebCore::ImageSource::uti): Deleted.
(WebCore::ImageSource::filenameExtension): Deleted.
(WebCore::ImageSource::accessibilityDescription): Deleted.
(WebCore::ImageSource::hotSpot): Deleted.
(WebCore::ImageSource::orientation): Deleted.
(WebCore::ImageSource::densityCorrectedSize): Deleted.
(WebCore::ImageSource::size): Deleted.
(WebCore::ImageSource::sourceSize): Deleted.
(WebCore::ImageSource::singlePixelSolidColor): Deleted.
(WebCore::ImageSource::maximumSubsamplingLevel): Deleted.
* Source/WebCore/platform/graphics/ImageSource.h:
* Source/WebCore/platform/graphics/NativeImage.h:
* Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp:
(WebCore::NativeImage::singlePixelSolidColor const):
* Source/WebCore/platform/graphics/cg/NativeImageCG.cpp:
(WebCore::NativeImage::singlePixelSolidColor const):
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label Jan 31, 2024
@shallawa shallawa force-pushed the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch from fd814ac to 5d5ba2e Compare January 31, 2024 21:17
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 5d5ba2e)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@shallawa shallawa mentioned this pull request Feb 6, 2024
Merged
cdumez
cdumez reviewed Feb 6, 2024
View reviewed changes
Source/WebCore/platform/graphics/BitmapImage.cpp
auto image = nativeImageForCurrentFrame();
if (!image)
return image;
auto nativeImage = nativeImageForCurrentFrame();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use an explicit smart pointer type instead of auto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in BitmapImageSource::preTransformedNativeImageAtIndex() in #20916.

Source/WebCore/platform/graphics/BitmapImage.cpp

FloatRect destRect(FloatPoint(), correctedSizeFloat);
FloatRect sourceRect(FloatPoint(), sourceSize);
auto buffer = ImageBuffer::create(size, RenderingPurpose::Unspecified, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in BitmapImageSource::preTransformedNativeImageAtIndex() in #20916

Source/WebCore/platform/graphics/BitmapImageMetadata.cpp
if (m_cachedFlags.contains(cachedFlag))
return cachedValue;

auto decoder = source().m_decoder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/BitmapImageMetadata.cpp

cachedValue = (*decoder.*functor)();
m_cachedFlags.add(cachedFlag);
source().didDecodeProperties(decoder->bytesDecodedToDetermineProperties());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You likely want protectedSource() here, which returns a smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/BitmapImageMetadata.cpp
if (m_cachedFlags.contains(cachedFlag))
return cachedValue;

auto nativeImage = const_cast<ImageSource&>(source()).frameImageAtIndexCacheIfNeeded(primaryFrameIndex());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/BitmapImageMetadata.cpp
if (m_cachedFlags.contains(CachedFlag::MaximumSubsamplingLevel))
return m_maximumSubsamplingLevel;

auto decoder = source().m_decoder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/BitmapImageMetadata.h
void dump(TextStream&) const;

private:
enum class CachedFlag {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: uint16_t

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/BitmapImageMetadata.h
MaximumSubsamplingLevel = 1 << 13,
};

ImageSource& source() const { return *m_source.get(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be named protectedSource() and return a RefPtr given that the m_source is a ThreadSafeWeakPtr and ThreadSafeWeakPtr::get() returns a smart pointer.

Ref<ImageSource> protectedSource() const { return m_source.get().releaseNonNull(); }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/Image.cpp
@@ -170,7 +170,7 @@ void Image::fillWithSolidColor(GraphicsContext& ctxt, const FloatRect& dstRect,

void Image::drawPattern(GraphicsContext& ctxt, const FloatRect& destRect, const FloatRect& tileRect, const AffineTransform& patternTransform, const FloatPoint& phase, const FloatSize& spacing, ImagePaintingOptions options)
{
auto tileImage = preTransformedNativeImageForCurrentFrame(options.orientation() == ImageOrientation::Orientation::FromImage);
auto tileImage = preTransformedNativeImageForCurrentFrame(options.orientation());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

Source/WebCore/platform/graphics/ImageSource.h
WEBCORE_EXPORT unsigned frameCount() const;
RepetitionCount repetitionCount() const { return m_metadata.repetitionCount(); }
DestinationColorSpace colorSpace() const { return m_metadata.colorSpace(); }
bool notSolidColor() const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notSolidColor() is an odd name for a boolean getter. We usually use a prefix, per coding style. E.g. isNotSolidColor(). The double negative is also not great, not sure if this is avoidable. Maybe hasSolidColor() and we reverse the bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed in #20916.

@shallawa
Copy link
Contributor Author

shallawa commented Feb 7, 2024

The plan is to submit the whole BitmapImage refactor PR #20916 as is without split. All the comments here will be addressed in #20916.

@shallawa shallawa closed this Feb 7, 2024
@shallawa shallawa deleted the eng/Move-image-metadata-queries-to-a-separate-class-named-BitmapImageMetadata branch March 21, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez left review comments

@smfr smfr Awaiting requested review from smfr

@heycam heycam Awaiting requested review from heycam

Assignees

@shallawa shallawa

Labels
Images For bugs in image handling.
Projects
None yet
Milestone
No milestone
4 participants
@shallawa @webkit-early-warning-system @cdumez @webkit-ews-buildbot