Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2521393002:
Prepare ManagedDisplayInfo to store touch calibration associated data (Closed)
|
Created:
4 years ago by malaykeshav Modified:
4 years ago CC:
chromium-reviews, tbuckley Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPrepare ManagedDisplayInfo to store touch calibration associated data
Touch calibration requires atleast 3 pairs of display and touch
points along with the total size of the display.
This change implements a struct to store the minimum touch
calibration associated data and an API for ManagedDisplayInfo
to store and retrieve this data when required.
Design Doc=go/cros-touch-calibration (Draft)
BUG=667921
COMPONENT=Display, Chrome OS, Managed Display Info
Committed: https://crrev.com/2ce4d1d5b71c39ca4cd55d2055e8114eb78502d8
Cr-Commit-Position: refs/heads/master@{#434791}
Patch Set 1 #
Total comments: 8
Patch Set 2 : Resolve Comments #
Total comments: 2
Patch Set 3 : nits #
Messages
Total messages: 35 (24 generated)
The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
malaykeshav@chromium.org changed reviewers: + oshima@chromium.org
PTAL. This is to allow the storage and retrieval of touch calibration associated data for a display.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
malaykeshav@chromium.org changed reviewers: + afakhry@chromium.org
Description was changed from ========== Prepare ManagedDisplayInfo to store touch calibration associated data Touch calibration requires atleast 3 pairs of display and touch points along with the total size of the display. This change implements a struct to store the minimum touch calibration associated data and an API for ManagedDisplayInfo to store and retrieve this data when required. BUG=667921 COMPONENT=Display, Chrome OS, Managed Display Info ========== to ========== Prepare ManagedDisplayInfo to store touch calibration associated data Touch calibration requires atleast 3 pairs of display and touch points along with the total size of the display. This change implements a struct to store the minimum touch calibration associated data and an API for ManagedDisplayInfo to store and retrieve this data when required. Design Doc=go/cros-touch-calibration (Draft) BUG=667921 COMPONENT=Display, Chrome OS, Managed Display Info ==========
ping
https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... File ui/display/manager/managed_display_info.h (right): https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info.h:31: using CalibrationPoints = std::array<CalibrationPointPair, 4>; how about CalibrationPointPairQuad ? https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info.h:268: // Sets/Gets the touch calibration data for the display. indent https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info.h:270: TouchCalibrationData const & https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... File ui/display/manager/managed_display_info_unittest.cc (right): https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info_unittest.cc:161: points[0] = pair[0]; std::make_pair
The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run
Resolved comments. PTAL https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... File ui/display/manager/managed_display_info.h (right): https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info.h:31: using CalibrationPoints = std::array<CalibrationPointPair, 4>; On 2016/11/23 at 21:52:42, oshima wrote: > how about CalibrationPointPairQuad ? Done https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info.h:268: // Sets/Gets the touch calibration data for the display. On 2016/11/23 at 21:52:42, oshima wrote: > indent Done https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info.h:270: TouchCalibrationData On 2016/11/23 at 21:52:42, oshima wrote: > const & Done https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... File ui/display/manager/managed_display_info_unittest.cc (right): https://codereview.chromium.org/2521393002/diff/1/ui/display/manager/managed_... ui/display/manager/managed_display_info_unittest.cc:161: points[0] = pair[0]; On 2016/11/23 at 21:52:42, oshima wrote: > std::make_pair Done
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
lgtm with nits https://codereview.chromium.org/2521393002/diff/20001/ui/display/manager/mana... File ui/display/manager/managed_display_info_unittest.cc (right): https://codereview.chromium.org/2521393002/diff/20001/ui/display/manager/mana... ui/display/manager/managed_display_info_unittest.cc:161: points[3] = std::make_pair(gfx::Point(190, 90), gfx::Point(189, 88)); nit: i believe you can also do points = { std::make_pair, std::make_par, ... .. }; up to you though. https://codereview.chromium.org/2521393002/diff/20001/ui/display/manager/mana... ui/display/manager/managed_display_info_unittest.cc:173: for (int i = 0; i < 4; i++) { can you use the arraysize instead of 4?
The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/11/24 at 00:36:40, oshima wrote: > lgtm with nits > > https://codereview.chromium.org/2521393002/diff/20001/ui/display/manager/mana... > File ui/display/manager/managed_display_info_unittest.cc (right): > > https://codereview.chromium.org/2521393002/diff/20001/ui/display/manager/mana... > ui/display/manager/managed_display_info_unittest.cc:161: points[3] = std::make_pair(gfx::Point(190, 90), gfx::Point(189, 88)); > nit: i believe you can also do > points = { > std::make_pair, > std::make_par, > ... > .. > }; > > up to you though. > > https://codereview.chromium.org/2521393002/diff/20001/ui/display/manager/mana... > ui/display/manager/managed_display_info_unittest.cc:173: for (int i = 0; i < 4; i++) { > can you use the arraysize instead of 4? done
The CQ bit was unchecked by malaykeshav@chromium.org
The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by malaykeshav@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org Link to the patchset: https://codereview.chromium.org/2521393002/#ps40001 (title: "nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by malaykeshav@chromium.org
The CQ bit was checked by malaykeshav@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480374830743320,
"parent_rev": "a8c0047a8e98edddc7c4cbf8586ad298193fc896", "commit_rev":
"b0b7f288c86843b51b9b4e8b34265ba987d501c2"}
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Prepare ManagedDisplayInfo to store touch calibration associated data Touch calibration requires atleast 3 pairs of display and touch points along with the total size of the display. This change implements a struct to store the minimum touch calibration associated data and an API for ManagedDisplayInfo to store and retrieve this data when required. Design Doc=go/cros-touch-calibration (Draft) BUG=667921 COMPONENT=Display, Chrome OS, Managed Display Info ========== to ========== Prepare ManagedDisplayInfo to store touch calibration associated data Touch calibration requires atleast 3 pairs of display and touch points along with the total size of the display. This change implements a struct to store the minimum touch calibration associated data and an API for ManagedDisplayInfo to store and retrieve this data when required. Design Doc=go/cros-touch-calibration (Draft) BUG=667921 COMPONENT=Display, Chrome OS, Managed Display Info Committed: https://crrev.com/2ce4d1d5b71c39ca4cd55d2055e8114eb78502d8 Cr-Commit-Position: refs/heads/master@{#434791} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/2ce4d1d5b71c39ca4cd55d2055e8114eb78502d8 Cr-Commit-Position: refs/heads/master@{#434791} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
