Summary: | shouldRespectImageOrientation should be a value in ImageOrientation | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||||||||
Component: | Images | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | commit-queue, dino, simon.fraser, thorton, webkit-bug-importer | ||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||
Bug Blocks: | 89052 | ||||||||||||||||||||||
Attachments: |
|
Description
Said Abou-Hallawa
2019-08-08 16:32:44 PDT
Created attachment 375863 [details]
Patch
Created attachment 375865 [details]
Patch
Created attachment 375868 [details]
Patch
Created attachment 375871 [details]
Patch
Created attachment 376089 [details]
Patch
Comment on attachment 376089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376089&action=review We're going to force call sites to be explicit about whether they want from-image. > Source/WebCore/platform/graphics/ImageOrientation.h:56 > + Default = None, > + > + EXIFFirst = OriginTopLeft, > + EXIFLast = OriginLeftBottom, > + > + First = FromImage, > + Last = EXIFLast Maybe moved these out and just define them where they get used. > Source/WebCore/platform/graphics/ImageOrientation.h:71 > + operator int() const { return static_cast<int>(m_orientation); } Remove this. > Source/WebCore/platform/graphics/ImageOrientation.h:-79 > - ImageOrientationEnum imageOrientation() { return m_orientation; } Keep this, call it .value()? Callers should switch on this. Created attachment 376177 [details]
Patch
Comment on attachment 376089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376089&action=review >> Source/WebCore/platform/graphics/ImageOrientation.h:56 >> + Last = EXIFLast > > Maybe moved these out and just define them where they get used. The first and last values are made private static constants inside the struct. >> Source/WebCore/platform/graphics/ImageOrientation.h:71 >> + operator int() const { return static_cast<int>(m_orientation); } > > Remove this. I changed it to be: operator Orientation() const { return m_orientation; } This can make this code possible: if (orientation == ImageOrientation::None) And this also: switch (orientation) { case ImageOrientation::None: // Compiler will complain if an enum value is missed or out of range value is used. ... } >> Source/WebCore/platform/graphics/ImageOrientation.h:-79 >> - ImageOrientationEnum imageOrientation() { return m_orientation; } > > Keep this, call it .value()? Callers should switch on this. I think the enum casting operator makes sense for this class since it is just a wrapper around the enum value. So I think if (orientation == ImageOrientation::None) Looks natural and it is better than if (orientation.value() == ImageOrientation::None) Or if (orientation == ImageOrientation(ImageOrientation::None)) The casting operator and the ImageOrientation(Orientation) constructor the hide fact that ImageOrientation is struct and make it look like an enum which is the common case for the ImageOrientation usage. Created attachment 376178 [details]
Patch
Created attachment 376181 [details]
Patch
Comment on attachment 376181 [details] Patch Rejecting attachment 376181 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 376181, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 5000 characters of output: e.xcodeproj/project.pbxproj.rej patching file Source/WebCore/css/CSSPrimitiveValueMappings.h patching file Source/WebCore/dom/DataTransfer.cpp patching file Source/WebCore/html/HTMLCanvasElement.cpp patching file Source/WebCore/loader/cache/CachedImage.cpp patching file Source/WebCore/page/DragController.cpp patching file Source/WebCore/platform/DragImage.cpp patching file Source/WebCore/platform/DragImage.h patching file Source/WebCore/platform/graphics/BitmapImage.cpp patching file Source/WebCore/platform/graphics/BitmapImage.h patching file Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp patching file Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h patching file Source/WebCore/platform/graphics/CustomPaintImage.cpp patching file Source/WebCore/platform/graphics/CustomPaintImage.h patching file Source/WebCore/platform/graphics/GeneratedImage.h patching file Source/WebCore/platform/graphics/GradientImage.cpp patching file Source/WebCore/platform/graphics/GradientImage.h patching file Source/WebCore/platform/graphics/GraphicsContext.cpp patching file Source/WebCore/platform/graphics/GraphicsContext.h patching file Source/WebCore/platform/graphics/GraphicsContextImpl.cpp patching file Source/WebCore/platform/graphics/Image.cpp patching file Source/WebCore/platform/graphics/Image.h patching file Source/WebCore/platform/graphics/ImageFrame.h patching file Source/WebCore/platform/graphics/ImageOrientation.cpp rm 'Source/WebCore/platform/graphics/ImageOrientation.cpp' patching file Source/WebCore/platform/graphics/ImageOrientation.h patching file Source/WebCore/platform/graphics/ImageSource.cpp patching file Source/WebCore/platform/graphics/NamedImageGeneratedImage.cpp patching file Source/WebCore/platform/graphics/NamedImageGeneratedImage.h patching file Source/WebCore/platform/graphics/NativeImage.h patching file Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm patching file Source/WebCore/platform/graphics/cairo/CairoOperations.cpp patching file Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp patching file Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp patching file Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp patching file Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp patching file Source/WebCore/platform/graphics/cg/NativeImageCG.cpp patching file Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp patching file Source/WebCore/platform/graphics/cg/PDFDocumentImage.h patching file Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp patching file Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp patching file Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h patching file Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp patching file Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.h patching file Source/WebCore/platform/graphics/win/Direct2DOperations.cpp patching file Source/WebCore/platform/graphics/win/ImageCGWin.cpp patching file Source/WebCore/platform/graphics/win/ImageCairoWin.cpp patching file Source/WebCore/platform/graphics/win/ImageDecoderDirect2D.cpp patching file Source/WebCore/platform/graphics/win/ImageDirect2D.cpp patching file Source/WebCore/platform/graphics/win/NativeImageDirect2D.cpp patching file Source/WebCore/platform/gtk/DragImageGtk.cpp patching file Source/WebCore/platform/image-decoders/ScalableImageDecoderFrame.h patching file Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp patching file Source/WebCore/platform/ios/DragImageIOS.mm patching file Source/WebCore/platform/mac/DragImageMac.mm patching file Source/WebCore/platform/win/DragImageCGWin.cpp patching file Source/WebCore/platform/win/DragImageCairoWin.cpp patching file Source/WebCore/platform/win/DragImageDirect2D.cpp patching file Source/WebCore/rendering/RenderBoxModelObject.cpp patching file Source/WebCore/rendering/RenderElement.cpp patching file Source/WebCore/rendering/RenderElement.h patching file Source/WebCore/rendering/RenderEmbeddedObject.cpp patching file Source/WebCore/rendering/RenderImage.cpp patching file Source/WebCore/rendering/RenderLayerBacking.cpp patching file Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp patching file Source/WebCore/rendering/style/RenderStyle.h patching file Source/WebCore/rendering/style/StyleRareInheritedData.h patching file Source/WebCore/svg/graphics/SVGImage.cpp patching file Source/WebCore/svg/graphics/SVGImage.h patching file Source/WebCore/svg/graphics/SVGImageForContainer.cpp patching file Source/WebCore/svg/graphics/SVGImageForContainer.h patching file Source/WebKit/ChangeLog Hunk #1 succeeded at 1 with fuzz 1. patching file Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Simon Fraser']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/12906694 Created attachment 376224 [details]
Patch
Comment on attachment 376224 [details] Patch Clearing flags on attachment: 376224 Committed r248657: <https://trac.webkit.org/changeset/248657> All reviewed patches have been landed. Closing bug. |