Bug 163696

Summary: Extract common code for computing byte count for images
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: ImagesAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bfulgham, cdumez, commit-queue, dino, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, simon.fraser, zalan
Priority: P2    
Version: Safari 10   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=163757
https://bugs.webkit.org/show_bug.cgi?id=163670
https://bugs.webkit.org/show_bug.cgi?id=163762
Bug Depends on: 163670    
Bug Blocks:    
Attachments:
Description Flags
Patch v1 simon.fraser: review-, simon.fraser: commit-queue-

David Kilzer (:ddkilzer)
Reported 2016-10-19 14:53:50 PDT
There are many places where we multiply IntSize.width() * IntSize.height() * 4, so we should pull this into a utility method. I chose Color.h mostly because it's where "typedef unsigned RGBA32" lives.
Attachments
Patch v1 (19.32 KB, patch)
2016-10-19 15:01 PDT, David Kilzer (:ddkilzer)
simon.fraser: review-
simon.fraser: commit-queue-
David Kilzer (:ddkilzer)
Comment 1 2016-10-19 15:01:02 PDT
Created attachment 292116 [details] Patch v1
Simon Fraser (smfr)
Comment 2 2016-10-19 15:22:21 PDT
Comment on attachment 292116 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=292116&action=review > Source/WebCore/platform/graphics/Color.h:402 > +inline unsigned numBytesWithColor(const IntSize& size) > +{ > + Checked<unsigned> result = Checked<unsigned>(size.area()) * sizeof(RGBA32); > + return result.unsafeGet(); > +} I don't think this is a good place, and encourages people to assume sizeof(pixel) == 4 but that might change. I think each image or buffer object should have a bytesSize() accessor.
David Kilzer (:ddkilzer)
Comment 3 2016-10-20 13:42:44 PDT
Broke out PDFDocumentImage.cpp changes into: Bug 163757: Use IntSize::unclampedArea() in PDFDocumentImage::updateCachedImageIfNeeded()
David Kilzer (:ddkilzer)
Comment 4 2016-10-20 15:34:05 PDT
Broke out use of checked arithmetic in IntSize::area() as: Bug 163762: IntSize::area() should used checked arithmetic
David Kilzer (:ddkilzer)
Comment 5 2016-10-20 15:35:29 PDT
Moving to RESOLVED/WONTFIX since there probably needs to be more discussion here before a change is made, and I'm not an area expert.
Note You need to log in before you can comment on or make changes to this bug.