Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1834243002:
Add 'P' to SampleApp to cycle through pixel geometries. (Closed)
|
Created:
4 years, 8 months ago by bungeman-skia Modified:
4 years, 8 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionAdd 'P' to SampleApp to cycle through pixel geometries.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1834243002
Committed: https://skia.googlesource.com/skia/+/5a59a422970eb63dd5af6baa797b25fba9dec5bb
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase #
Messages
Total messages: 20 (11 generated)
Description was changed from ========== Add 'P' to SampleApp to cycle through pixel geometries. ========== to ========== Add 'P' to SampleApp to cycle through pixel geometries. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add 'P' to SampleApp to cycle through pixel geometries. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add 'P' to SampleApp to cycle through pixel geometries. ==========
bungeman@google.com changed reviewers: + mtklein@google.com, reed@google.com
I found myself wanting to interactively influence the pixel geometry in SampleApp.
The CQ bit was checked by bungeman@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834243002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834243002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/1834243002/diff/1/samplecode/SampleApp.cpp File samplecode/SampleApp.cpp (right): https://codereview.chromium.org/1834243002/diff/1/samplecode/SampleApp.cpp#ne... samplecode/SampleApp.cpp:478: {SkPixelGeometry::kUnknown_SkPixelGeometry, "Flat", "{Flat} " }, I presume Flat means ... unknown?
https://codereview.chromium.org/1834243002/diff/1/samplecode/SampleApp.cpp File samplecode/SampleApp.cpp (right): https://codereview.chromium.org/1834243002/diff/1/samplecode/SampleApp.cpp#ne... samplecode/SampleApp.cpp:478: {SkPixelGeometry::kUnknown_SkPixelGeometry, "Flat", "{Flat} " }, On 2016/03/31 11:04:13, reed1 wrote: > I presume Flat means ... unknown? Right here I'm not stating that the created surface should have some unknown pixel geometry or that the eventual display's pixel geometry is unknown, but stating that I'm creating a surface where the pixel geometry is explicitly flat. Using the term "Flat" here in the UI is stating that the surface's pixel geometry is flat, it most assuredly is not "unknown". Indeed, the "Mixed" actually means something more like unknown, since the pixelGeometry in the above PixelGeometryState is just a dummy, it isn't used (the legacy default is what is used). From the user's perspective, it's kUnknown that's misnamed. SkPixelGeometry is used to specify the pixel geometry of the SkSurface, it doesn't need to reflect the user's knowledge of where the SkSurface will eventually end up. In other words, by specifying the SkPixelGeometry on a surface the user is definitively configuring the surface, in which case stating 'I don't know what you should be' isn't very clear. 'kFlat_SkPixelGeometry' or even 'kNone_SkPixelGeometry' would be better names. If some user doesn't know the pixel geometry of the eventual display device then of course they should explicitly state 'flat' as opposed to just 'unknown'. The naming of this is somewhat a matter of perspective. The use of 'kUnknown' reflects what the names would be if one were to query a device for its pixel geometry. (Though in this case 'kUnknown' would probably better be called 'kOther'.) Since in Skia these enum names are only ever used to create a surface (not query a device) I believe they should have the perspective of telling the surface what its pixel geometry is.
The CQ bit was checked by bungeman@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834243002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834243002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...) Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...) Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)
Description was changed from ========== Add 'P' to SampleApp to cycle through pixel geometries. ========== to ========== Add 'P' to SampleApp to cycle through pixel geometries. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by bungeman@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com Link to the patchset: https://codereview.chromium.org/1834243002/#ps20001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1834243002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1834243002/20001
Message was sent while issue was closed.
Description was changed from ========== Add 'P' to SampleApp to cycle through pixel geometries. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add 'P' to SampleApp to cycle through pixel geometries. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/5a59a422970eb63dd5af6baa797b25fba9dec5bb ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/5a59a422970eb63dd5af6baa797b25fba9dec5bb |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
