[chromium/mac] Make spelling indicator HighDPI
Created attachment 168856 [details] Patch
Attachment 168856 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:643: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Note that this patch is much simpler than it looks. It does the following things: 1. Undo the forking of the function code done in the patch linked in the changelog. 2. Just add the else block that contains the comment " // Pattern: a b c c b a" This probably needs new baselines too (i.e. this isn't the final patch, but the code should be final); I'll look into that tomorrow. See http://crbug.com/156022 for where the color values come from.
Created attachment 168979 [details] Patch
Attachment 168979 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:643: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 168982 [details] Patch
Attachment 168982 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:643: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 168982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168982&action=review Fix the ChangeLog, satisfy the bots, the rest is at your discretion. > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:604 > + ASSERT(deviceScaleFactor > 0); Maybe this could be ASSERT(deviceScaleFactor == 1 || deviceScaleFactor == 2) (see below). > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:640 > + } else { Maybe: } else if (deviceScaleFactor == 2) { ... } else { ASSERT_NOT_REACHED(); } That way, if we ever try to support some future DPI, we can find the problems quickly. > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:663 > + // Pattern: a b c c b a > + // d e f f e d > + // g h j j h g > + // k l m m l k > + // n o p p o n > + // q r s s r q > + for (int x = 0; x < colPixels; ++x) { > + uint32_t* row = misspellBitmap[index]->getAddr32(0, x); > + row[0] = colors[index][x * 3]; > + row[1] = colors[index][x * 3 + 1]; > + row[2] = colors[index][x * 3 + 2]; > + row[3] = colors[index][x * 3 + 2]; > + row[4] = colors[index][x * 3 + 1]; > + row[5] = colors[index][x * 3]; > + row[6] = transparentColor; > + row[7] = transparentColor; Nit: this looks like a b c c b a d e f f e d g h j j h g g h j j h g d e f f e d a b c c b a to me.
Created attachment 168986 [details] Patch
(In reply to comment #8) > (From update of attachment 168982 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=168982&action=review > > Fix the ChangeLog, Done. > satisfy the bots I think the style bot is wrong here; the pixels would look strange with a 4 indent (not aligned with the previous line) > > > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:604 > > + ASSERT(deviceScaleFactor > 0); > > Maybe this could be ASSERT(deviceScaleFactor == 1 || deviceScaleFactor == 2) (see below). Done. > > > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:640 > > + } else { > > Maybe: > > } else if (deviceScaleFactor == 2) { > ... > } else { > ASSERT_NOT_REACHED(); > } > > That way, if we ever try to support some future DPI, we can find the problems quickly. Done. (also in the non-mac code) > > > Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:663 > > + // Pattern: a b c c b a > > + // d e f f e d > > + // g h j j h g > > + // k l m m l k > > + // n o p p o n > > + // q r s s r q > > + for (int x = 0; x < colPixels; ++x) { > > + uint32_t* row = misspellBitmap[index]->getAddr32(0, x); > > + row[0] = colors[index][x * 3]; > > + row[1] = colors[index][x * 3 + 1]; > > + row[2] = colors[index][x * 3 + 2]; > > + row[3] = colors[index][x * 3 + 2]; > > + row[4] = colors[index][x * 3 + 1]; > > + row[5] = colors[index][x * 3]; > > + row[6] = transparentColor; > > + row[7] = transparentColor; > > Nit: this looks like > > a b c c b a > d e f f e d > g h j j h g > g h j j h g > d e f f e d > a b c c b a > > to me. No, it's not x-symmetrical.
Attachment 168986 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:643: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:645: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 168982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168982&action=review >>> Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:663 >>> + row[7] = transparentColor; >> >> Nit: this looks like >> >> a b c c b a >> d e f f e d >> g h j j h g >> g h j j h g >> d e f f e d >> a b c c b a >> >> to me. > > No, it's not x-symmetrical. Yeah, I misunderstood. Sorry for the noise.
Comment on attachment 168986 [details] Patch We agreed style-bot can be ignored. Ignore other bots at your peril. :) r=me
Comment on attachment 168986 [details] Patch Clearing flags on attachment: 168986 Committed r131483: <http://trac.webkit.org/changeset/131483>
All reviewed patches have been landed. Closing bug.