Skip to content
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.

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

fix: port OSR code to new viz compositor codepath #17538

Merged
merged 23 commits into from Apr 17, 2019

Conversation

Projects
None yet
7 participants
@brenca
Copy link
Member

commented Mar 25, 2019

Description of Change

Fixes #16983.

There is an ongoing effort to upstream a large chunk of the patch to chromium (one of the guys from CEF is doing the upstreaming, and the chromium folks indicated that they are open to the patch).

TODO:

  • Windows - Software rendering
  • Windows - GPU rendering
  • MacOS - Software rendering
  • MacOS - GPU rendering
  • Linux - Software rendering
  • Linux - GPU rendering
  • Backport to 5-0-x

Checklist

Release Notes

Notes: Fixed offscreen rendering not working with viz compositor.

@brenca brenca requested a review from electron/wg-upgrades as a code owner Mar 25, 2019

@electron-cation electron-cation bot added new-pr 🌱 and removed new-pr 🌱 labels Mar 25, 2019

@brenca brenca force-pushed the brenca/viz-osr branch from cd5ec08 to 9fe2379 Mar 26, 2019

@brenca brenca requested review from codebytere and zcbenz Mar 27, 2019

@brenca brenca changed the title fix: port OSR code to new viz compositor codepath [WIP] fix: port OSR code to new viz compositor codepath Mar 27, 2019

@electron electron deleted a comment from trop bot Mar 27, 2019

@@ -0,0 +1,121 @@
// Copyright (c) 2016 GitHub, Inc.

This comment has been minimized.

Copy link
@codebytere

codebytere Apr 2, 2019

Member

nit: update copyright year

@codebytere

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

For a pull request this large, it's challenging to inspect every nook and cranny as thoroughly as i'd like but based on this writeup and my understanding of the code at hand this seems to be sufficient for us to merge. I don't see any glaring issues and it's (imo) more important to release this to consumers such that the beta cycle surfaces any bugs that may be present therein and that weren't caught in initial development.

@ckerr

ckerr approved these changes Apr 3, 2019

@ckerr ckerr added target/5-0-x and removed target/5-0-x labels Apr 3, 2019

@trop

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

A maintainer has manually backported this PR to "5-0-x", please check out #17572

@nornagon

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

one of the guys from CEF

btw, I'm pretty sure there's only one guy from CEF: Marshall Greenblatt :)

@brenca

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

@nornagon Well, most of the viz osr work is actually done by Alexander Guettler, we have group PM with him, Marshall Greenblatt and @codebytere where we discussed how things are going. See https://bitbucket.org/chromiumembedded/cef/issues/2575

@jkleinsc

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@brenca can you rebase this PR?

@brenca brenca force-pushed the brenca/viz-osr branch from 19ea77b to d1c2b77 Apr 16, 2019

@codebytere codebytere merged commit 81bf158 into master Apr 17, 2019

15 checks passed

Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190417.14 succeeded
Details
electron-arm64-testing Build #20190417.14 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Apr 17, 2019

Release Notes Persisted

Fixed offscreen rendering not working with viz compositor.

@codebytere codebytere deleted the brenca/viz-osr branch Apr 17, 2019

Kiku-git added a commit to Kiku-git/electron that referenced this pull request May 16, 2019

fix: port OSR code to new viz compositor codepath (electron#17538)
* fix: make OSR work with viz compositor

* fix: update OSR patch

* fix: update patch again

* fix: update viz_osr.patch for macOS

* fix: gn check warnings

* chore: no need to change SoftwareOutputDeviceWinProxy

* chore: add check in case we missed something

* fix: consider scale factor when compare size

* fix: make GPU OSR work

* fix: autofill popups with OSR

* chore: use UNIX line ending for osr_video_consumer

* chore: code is already in defined(OS_MACOSX)

* fix: share same OSR implementation on macOS

This should also fix the crash when there is navigation on macOS.

* test: osr window should not crash after navigation

* fix: make osr work on Mac properly

* fix: software osr on windows

* fix: software osr on Linux

* fix: compilation error introduced with rebase

* fix: split local surface id allocation into two

* Update osr_host_display_client_mac.mm

* chore: update copyright year

* fix: update patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.