Summary: | [iOS] Make document marker assets not specific to particular scale factors | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||
Component: | Text | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dino, jonlee, simon.fraser, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Myles C. Maxfield
2014-08-06 14:16:48 PDT
Created attachment 236138 [details]
Patch
Comment on attachment 236138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236138&action=review > Source/WebCore/platform/ios/wak/WKGraphics.mm:96 > + NSString *fileName = scaleFactor == 1 ? [NSString stringWithUTF8String:imageFile] : [NSString stringWithFormat:@"%s@%dx", imageFile, scaleFactor]; You've removed the fallback that used to be here. I think it would be better to reinstate the fallback, but assert when the correct image is not found. Created attachment 236139 [details]
Patch
Comment on attachment 236139 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236139&action=review > Source/WebCore/platform/ios/wak/WKGraphics.mm:116 > + NSData *imageData = nullptr; > + for (unsigned scaleFactor = wkGetScreenScaleFactor(); scaleFactor > 0; --scaleFactor) { > + imageData = [NSData dataWithContentsOfFile:imageResourcePath(image_file, scaleFactor)]; > + ASSERT(scaleFactor != wkGetScreenScaleFactor() || imageData); Now you're just fetching the 1x image. Created attachment 236143 [details]
Patch
Comment on attachment 236143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236143&action=review > Source/WebCore/ChangeLog:9 > + No new tests, because on iOS I can only test whether or not document > + markers should be rendered, rather than whether or not they were rendered I don't think you need to justify the lack of tests. > Source/WebCore/platform/ios/wak/WKGraphics.mm:116 > + ASSERT(scaleFactor != wkGetScreenScaleFactor() || imageData); I think an if (imageData) break; would be a little easier to read. Comment on attachment 236143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236143&action=review >> Source/WebCore/ChangeLog:9 >> + markers should be rendered, rather than whether or not they were rendered > > I don't think you need to justify the lack of tests. Done. >> Source/WebCore/platform/ios/wak/WKGraphics.mm:116 >> + ASSERT(scaleFactor != wkGetScreenScaleFactor() || imageData); > > I think an if (imageData) break; would be a little easier to read. Done. |