Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2848443004:
Allow Windows to return a monitor out of the first quadrant (Closed)
|
Created:
3 years, 7 months ago by Hzj_jie Modified:
3 years, 7 months ago Reviewers:
Sergey Ulanov CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAllow Windows to return a monitor out of the first quadrant
Windows may return a DesktopCoordinate out of the first quadrant
(x >= 0 && y >= 0), this typically happens when the primary and secondary
monitors are swapped. i.e. The secondary monitor is on the left but the primary
one is on the right.
This change "moves" the entire screen from any quadrant to the (0, 0), so we can
capture the monitors in other quadrants.
BUG=chromium:715689
Review-Url: https://codereview.webrtc.org/2848443004
Cr-Original-Commit-Position: refs/heads/master@{#17935}
Committed: https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d60acb1a71e81b521
Review-Url: https://codereview.webrtc.org/2848443004
Cr-Commit-Position: refs/heads/master@{#17938}
Committed: https://chromium.googlesource.com/external/webrtc/+/419c7427153f3e72ff2a0edf799faf3b1f07c845
Patch Set 1 #
Total comments: 12
Patch Set 2 : Resolve review comments #Patch Set 3 : TranslateRect() should only be executed once #
Messages
Total messages: 43 (34 generated)
The CQ bit was checked by zijiehe@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.webrtc.org/...
Description was changed from ========== Allow Windows to return a monitor out of the first quarant Windows may return a DesktopCoordinate out of the first quarant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=715689 ========== to ========== Allow Windows to return a monitor out of the first quarant Windows may return a DesktopCoordinate out of the first quarant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=715689 ==========
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
Description was changed from ========== Allow Windows to return a monitor out of the first quarant Windows may return a DesktopCoordinate out of the first quarant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=715689 ========== to ========== Allow Windows to return a monitor out of the first quarant Windows may return a DesktopCoordinate out of the first quarant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 ==========
Description was changed from ========== Allow Windows to return a monitor out of the first quarant Windows may return a DesktopCoordinate out of the first quarant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 ========== to ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 ==========
The CQ bit was checked by zijiehe@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.webrtc.org/...
zijiehe@chromium.org changed reviewers: - sergeyu@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #1 (id:1) has been deleted
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Patchset #1 (id:20001) has been deleted
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc (right): https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc:155: void DxgiAdapterDuplicator::Translate(int32_t dx, int32_t dy) { TranslateRect()? https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc:155: void DxgiAdapterDuplicator::Translate(int32_t dx, int32_t dy) { Use DesktopVector to pass the parameters https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h (right): https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h:188: // executed in DoInitialize() only. Suggest rewording it a bit to simplify: // Moves |desktop_rect_| and all underlying |duplicators_|, putting top // left corner of the desktop at (0, 0). This is necessary because // DXGI_OUTPUT_DESC may return negative desktop coordinates. // Called from DoInitialize() after all DxgiAdapterDuplicator and // DxgiOutputDuplicator instances are initialized. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h:189: // This comment makes this piece of logic deserve a standalone function. I don't think you need this sentence. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h:190: void Translate(); TranslateDesktopRect()? https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h (right): https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h:79: void Translate(int32_t dx, int32_t dy); Maybe call it TranslateRect()? Also make it take DesktopVector instead of dx & dy separately.
The CQ bit was checked by zijiehe@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.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc (right): https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc:155: void DxgiAdapterDuplicator::Translate(int32_t dx, int32_t dy) { On 2017/04/28 01:15:02, Sergey Ulanov wrote: > TranslateRect()? Done. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc:155: void DxgiAdapterDuplicator::Translate(int32_t dx, int32_t dy) { On 2017/04/28 01:15:02, Sergey Ulanov wrote: > Use DesktopVector to pass the parameters Done. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h (right): https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h:188: // executed in DoInitialize() only. On 2017/04/28 01:15:02, Sergey Ulanov wrote: > Suggest rewording it a bit to simplify: > > // Moves |desktop_rect_| and all underlying |duplicators_|, putting top > // left corner of the desktop at (0, 0). This is necessary because > // DXGI_OUTPUT_DESC may return negative desktop coordinates. > // Called from DoInitialize() after all DxgiAdapterDuplicator and > // DxgiOutputDuplicator instances are initialized. Done. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h:189: // This comment makes this piece of logic deserve a standalone function. On 2017/04/28 01:15:02, Sergey Ulanov wrote: > I don't think you need this sentence. Done. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h:190: void Translate(); On 2017/04/28 01:15:02, Sergey Ulanov wrote: > TranslateDesktopRect()? Done. https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h (right): https://codereview.webrtc.org/2848443004/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h:79: void Translate(int32_t dx, int32_t dy); On 2017/04/28 01:15:02, Sergey Ulanov wrote: > Maybe call it TranslateRect()? > Also make it take DesktopVector instead of dx & dy separately. Done.
lgtm
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493412045699970,
"parent_rev": "281aecdca6b6e4cabd028698fffcf5ada6f85dd8", "commit_rev":
"049ec71e657eef3d15e0f15d60acb1a71e81b521"}
Message was sent while issue was closed.
Description was changed from ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 ========== to ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 Review-Url: https://codereview.webrtc.org/2848443004 Cr-Commit-Position: refs/heads/master@{#17935} Committed: https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d6... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d6...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:60001) has been created in https://codereview.chromium.org/2852783002/ by zijiehe@chromium.org. The reason for reverting is: TranslateRect() is placed in a wrong position..
Message was sent while issue was closed.
Description was changed from ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 Review-Url: https://codereview.webrtc.org/2848443004 Cr-Commit-Position: refs/heads/master@{#17935} Committed: https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d6... ========== to ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 Review-Url: https://codereview.webrtc.org/2848443004 Cr-Commit-Position: refs/heads/master@{#17935} Committed: https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d6... ==========
The CQ bit was checked by zijiehe@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.webrtc.org/...
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 zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.webrtc.org/2848443004/#ps80001 (title: "TranslateRect() should only be executed once")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493420739358370,
"parent_rev": "23c65b6a1972ecb88aa0f54dd4b3d3b42c9b618c", "commit_rev":
"419c7427153f3e72ff2a0edf799faf3b1f07c845"}
Message was sent while issue was closed.
Description was changed from ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 Review-Url: https://codereview.webrtc.org/2848443004 Cr-Commit-Position: refs/heads/master@{#17935} Committed: https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d6... ========== to ========== Allow Windows to return a monitor out of the first quadrant Windows may return a DesktopCoordinate out of the first quadrant (x >= 0 && y >= 0), this typically happens when the primary and secondary monitors are swapped. i.e. The secondary monitor is on the left but the primary one is on the right. This change "moves" the entire screen from any quadrant to the (0, 0), so we can capture the monitors in other quadrants. BUG=chromium:715689 Review-Url: https://codereview.webrtc.org/2848443004 Cr-Original-Commit-Position: refs/heads/master@{#17935} Committed: https://chromium.googlesource.com/external/webrtc/+/049ec71e657eef3d15e0f15d6... Review-Url: https://codereview.webrtc.org/2848443004 Cr-Commit-Position: refs/heads/master@{#17938} Committed: https://chromium.googlesource.com/external/webrtc/+/419c7427153f3e72ff2a0edf7... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/external/webrtc/+/419c7427153f3e72ff2a0edf7... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
