Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upfeat(window): bind hotkey to trigger capture event #20315
Merged
+36
−2
Conversation
highfive
commented
Mar 16, 2018
|
Heads up! This PR modifies the following files:
|
|
This looks fantastic! Thank you for splitting the commits in a very readable way. |
|
|
bors-servo
added a commit
that referenced
this pull request
Mar 16, 2018
feat(window): bind hotkey to trigger capture event <!-- Please describe your changes on the following line: --> Relates to #20295. This PR intends to expose additional hotkey to window to allow capture webrender. Internally it adds one new `WindowEvent::CaptureWebRender` for those purpose. I took some liberty to make some decisions around which need to be reviewed & updated in PR. - `Ctrl-shift-3` is binded to hotkey to follow described in Gecko's behavior. Is it good to go? - Maybe do not need to create new event `CaptureWebRender` but reuse `ToggleWebRenderDebug`, having additional `WebRenderDebugOption` values? : This sounds more right path for me, but `capture` isn't really `toggle` behavior to include capture into it. - Capturing will create `capture_webrender` in cwd, creates new folder inside each time new capture stored : Maybe it'd better to expose new cmdline args allow overrides, or some better way else. I took the simple approach to generate path without asking for it. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20295 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - This change has manually verified on local machines (mac, windows, linux). <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20315) <!-- Reviewable:end -->
|
|
|
Wonderful, thank you @kwonoj for addressing it so quickly! |
|
@kvark TIL, didn't know that. Let me try if I can trivially introduce it as well. |
bors-servo
added a commit
that referenced
this pull request
Mar 23, 2018
feat(capture_webrender): write webrender revision into text <!-- Please describe your changes on the following line: --> Relates to #20315 (comment). This PR try to generate `wr.txt` when trigger webrender capture. By reading gecko's implementation at [here](https://github.com/mozilla/gecko-dev/blob/3b8e63c66ae1989cfc2c7fb48ca9e025a3828e74/gfx/doc/README.webrender#L53), it seems gecko's build script generates txt file for containing revision of webrender and read it each time trigger capturing. In this PR tries to similar in cruxwise with small differences: - `cargo build` reads `cargo.lock`, export it into `${OUT_DIR}/`, included via macro in build time. - when capturing triggered, those revision will be written as `wr.txt`. Probably point of discussion & need to be updated in PR if necessary: ~- Is it acceptable `mach` generates module file on build bootstrapping? Should there be other recommendation?~ Now cargo build takes care of generation. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20295 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - This PR manually verified on local mac OS machine. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20320) <!-- Reviewable:end -->
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Mar 23, 2018
…on into text (from kwonoj:feat-wr-revision); r=jdm <!-- Please describe your changes on the following line: --> Relates to servo/servo#20315 (comment). This PR try to generate `wr.txt` when trigger webrender capture. By reading gecko's implementation at [here](https://github.com/mozilla/gecko-dev/blob/3b8e63c66ae1989cfc2c7fb48ca9e025a3828e74/gfx/doc/README.webrender#L53), it seems gecko's build script generates txt file for containing revision of webrender and read it each time trigger capturing. In this PR tries to similar in cruxwise with small differences: - `cargo build` reads `cargo.lock`, export it into `${OUT_DIR}/`, included via macro in build time. - when capturing triggered, those revision will be written as `wr.txt`. Probably point of discussion & need to be updated in PR if necessary: ~- Is it acceptable `mach` generates module file on build bootstrapping? Should there be other recommendation?~ Now cargo build takes care of generation. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20295 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - This PR manually verified on local mac OS machine. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 91398cf559ddeec8974e04b0a92e464669436177 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 9751ea51ab25079d0f1ac3d0f228bb97b0a61568
This was referenced Apr 24, 2018
bors-servo
added a commit
that referenced
this pull request
Apr 24, 2018
fix(browser): do not omit unexpected keyevent <!-- Please describe your changes on the following line: --> - closes #20681 This PR intends to fix regression caused by changes I created in #20315. In code, it matches keyevent aggressively for any pattern includes `Some('3')`, ends up actual key event does not bubbles up. This PR applies correct pattern guard to pick up specific keyevent only, other events falls back to default patterns. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [ ] `./mach build-geckolib` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20681 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - manually verified 1. typing `3` in input field works 2. ctrl-shift-3 create webrender capture (verified on mac os) <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20687) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this pull request
Apr 24, 2018
fix(browser): do not omit unexpected keyevent <!-- Please describe your changes on the following line: --> - closes #20681 This PR intends to fix regression caused by changes I created in #20315. In code, it matches keyevent aggressively for any pattern includes `Some('3')`, ends up actual key event does not bubbles up. This PR applies correct pattern guard to pick up specific keyevent only, other events falls back to default patterns. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [ ] `./mach build-geckolib` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20681 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - manually verified 1. typing `3` in input field works 2. ctrl-shift-3 create webrender capture (verified on mac os) <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20687) <!-- Reviewable:end -->
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 2, 2019
…on into text (from kwonoj:feat-wr-revision); r=jdm <!-- Please describe your changes on the following line: --> Relates to servo/servo#20315 (comment). This PR try to generate `wr.txt` when trigger webrender capture. By reading gecko's implementation at [here](https://github.com/mozilla/gecko-dev/blob/3b8e63c66ae1989cfc2c7fb48ca9e025a3828e74/gfx/doc/README.webrender#L53), it seems gecko's build script generates txt file for containing revision of webrender and read it each time trigger capturing. In this PR tries to similar in cruxwise with small differences: - `cargo build` reads `cargo.lock`, export it into `${OUT_DIR}/`, included via macro in build time. - when capturing triggered, those revision will be written as `wr.txt`. Probably point of discussion & need to be updated in PR if necessary: ~- Is it acceptable `mach` generates module file on build bootstrapping? Should there be other recommendation?~ Now cargo build takes care of generation. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20295 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - This PR manually verified on local mac OS machine. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 91398cf559ddeec8974e04b0a92e464669436177 UltraBlame original commit: cf3b8e9c9c2a290b09376c5e2b720fe1124eeacb
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 2, 2019
…on into text (from kwonoj:feat-wr-revision); r=jdm <!-- Please describe your changes on the following line: --> Relates to servo/servo#20315 (comment). This PR try to generate `wr.txt` when trigger webrender capture. By reading gecko's implementation at [here](https://github.com/mozilla/gecko-dev/blob/3b8e63c66ae1989cfc2c7fb48ca9e025a3828e74/gfx/doc/README.webrender#L53), it seems gecko's build script generates txt file for containing revision of webrender and read it each time trigger capturing. In this PR tries to similar in cruxwise with small differences: - `cargo build` reads `cargo.lock`, export it into `${OUT_DIR}/`, included via macro in build time. - when capturing triggered, those revision will be written as `wr.txt`. Probably point of discussion & need to be updated in PR if necessary: ~- Is it acceptable `mach` generates module file on build bootstrapping? Should there be other recommendation?~ Now cargo build takes care of generation. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20295 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - This PR manually verified on local mac OS machine. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 91398cf559ddeec8974e04b0a92e464669436177 UltraBlame original commit: cf3b8e9c9c2a290b09376c5e2b720fe1124eeacb
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 2, 2019
…on into text (from kwonoj:feat-wr-revision); r=jdm <!-- Please describe your changes on the following line: --> Relates to servo/servo#20315 (comment). This PR try to generate `wr.txt` when trigger webrender capture. By reading gecko's implementation at [here](https://github.com/mozilla/gecko-dev/blob/3b8e63c66ae1989cfc2c7fb48ca9e025a3828e74/gfx/doc/README.webrender#L53), it seems gecko's build script generates txt file for containing revision of webrender and read it each time trigger capturing. In this PR tries to similar in cruxwise with small differences: - `cargo build` reads `cargo.lock`, export it into `${OUT_DIR}/`, included via macro in build time. - when capturing triggered, those revision will be written as `wr.txt`. Probably point of discussion & need to be updated in PR if necessary: ~- Is it acceptable `mach` generates module file on build bootstrapping? Should there be other recommendation?~ Now cargo build takes care of generation. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20295 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ - This PR manually verified on local mac OS machine. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 91398cf559ddeec8974e04b0a92e464669436177 UltraBlame original commit: cf3b8e9c9c2a290b09376c5e2b720fe1124eeacb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
kwonoj commentedMar 16, 2018
•
edited by SimonSapin
Relates to #20295.
This PR intends to expose additional hotkey to window to allow capture webrender. Internally it adds one new
WindowEvent::CaptureWebRenderfor those purpose. I took some liberty to make some decisions around which need to be reviewed & updated in PR.Ctrl-shift-3is binded to hotkey to follow described in Gecko's behavior. Is it good to go?CaptureWebRenderbut reuseToggleWebRenderDebug, having additionalWebRenderDebugOptionvalues?: This sounds more right path for me, but
captureisn't reallytogglebehavior to include capture into it.capture_webrenderin cwd, creates new folder inside each time new capture stored: Maybe it'd better to expose new cmdline args allow overrides, or some better way else. I took the simple approach to generate path without asking for it.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is