Skip to content

Implement WebGLContext resize #519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 3, 2016
Merged

Implement WebGLContext resize #519

merged 1 commit into from
Nov 3, 2016

Conversation

MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented Nov 3, 2016 •
edited
Loading

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

  • Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
  • Support WebGL apps that change the canvas size & viewport on window.onresize

This change is Reviewable

emilio
emilio approved these changes Nov 3, 2016
View reviewed changes
Copy link
Member

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me, thanks for doing it!

I'd like @glennw to sign-off the device-touching code, if that's possible.

webrender/src/texture_cache.rs
@@ -779,6 +779,13 @@ impl TextureCache {
})
}

pub fn add_raw_remove(&mut self, id: TextureId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_raw_remove sounds a bit weird, though it follows the convention. I can't think of an immediately better name now, so I guess this is ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it does follow the convention. Perhaps, the convention should be revised? as a separate PR

kvark
kvark reviewed Nov 3, 2016
View reviewed changes
webrender/src/resource_cache.rs
let prev_texture_id = self.webgl_textures.get(&id).unwrap();

// Remove existing cache if texture id has changed
if !prev_texture_id.eq(&texture_id) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason !eq() and clone() are used instead of != and * (since this is Copy)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize that it is Copy;)

webrender/src/texture_cache.rs
@@ -779,6 +779,13 @@ impl TextureCache {
})
}

pub fn add_raw_remove(&mut self, id: TextureId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it does follow the convention. Perhaps, the convention should be revised? as a separate PR

@glennw
Copy link
Member

glennw commented Nov 3, 2016

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit d8bcc18 has been approved by glennw

@bors-servo
Copy link
Contributor

⌛ Testing commit d8bcc18 with merge 49dfb69...

bors-servo pushed a commit that referenced this pull request Nov 3, 2016
Auto merge of #519 - MortimerGoro:webgl_resize, r=glennw
49dfb69 
Implement WebGLContext resize

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:
- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/519)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit d8bcc18 into servo:master Nov 3, 2016
@bors-servo bors-servo mentioned this pull request Nov 3, 2016
Merged
@MortimerGoro MortimerGoro mentioned this pull request Nov 4, 2016
Merged
5 tasks
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 5, 2016
Auto merge of #14075 - MortimerGoro:webgl-resize, r=emilio
df3d72b 
Implement WebGLContext resize

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/14075)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 9, 2016
Auto merge of #14075 - MortimerGoro:webgl-resize, r=emilio
fad2164 
Implement WebGLContext resize

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/14075)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 28, 2016
Auto merge of #14075 - MortimerGoro:webgl-resize, r=emilio
94cdb3c 
Implement WebGLContext resize

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/14075)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Dec 1, 2016
Auto merge of #14075 - MortimerGoro:webgl-resize, r=MortimerGoro
94eefc4 
Implement WebGLContext resize

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/14075)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 4, 2017
@MortimerGoro
servo: Merge #14075 - Implement WebGLContext resize (from MortimerGor…
bf25e81 
…o:webgl-resize); r=MortimerGoro

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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: 94eefc4001e0998fcea5a35943da73624ea82b13
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
@marco-c
servo: Merge #14075 - Implement WebGLContext resize (from MortimerGor…
d2767e0 
…o:webgl-resize); r=MortimerGoro

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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: 94eefc4001e0998fcea5a35943da73624ea82b13

UltraBlame original commit: fc18c37273eda7200f883473b7409526bb03a592
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
@marco-c
servo: Merge #14075 - Implement WebGLContext resize (from MortimerGor…
e7e861e 
…o:webgl-resize); r=MortimerGoro

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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: 94eefc4001e0998fcea5a35943da73624ea82b13

UltraBlame original commit: fc18c37273eda7200f883473b7409526bb03a592
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
@marco-c
servo: Merge #14075 - Implement WebGLContext resize (from MortimerGor…
25c7492 
…o:webgl-resize); r=MortimerGoro

<!-- Please describe your changes on the following line: -->

Related PR: servo/webrender#519

Implement WebGLContext resize (canvas.width & canvas.height). I have tested:

- Fixes WebGL apps that initialize canvas size after calling canvas.getContext("webgl")
- Support WebGL apps that change the canvas size & viewport on window.onresize

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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: 94eefc4001e0998fcea5a35943da73624ea82b13

UltraBlame original commit: fc18c37273eda7200f883473b7409526bb03a592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio emilio approved these changes

+1 more reviewer

@kvark kvark kvark left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MortimerGoro @glennw @bors-servo @kvark @emilio