On Sat, 20 Mar 2021 13:40:51 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> This is a fix for a long-standing bug where the D3D pipeline will stop >> rendering when a Windows remote desktop session is disconnected and then >> reconnected. >> >> A preliminary Draft PR #315 by @Schmidor was a good first step in solving >> this. I took that and continued the work in my Draft PR #403. It is now >> ready for formal review in this new PR. You can see PR #403 for details on >> the history of the changes. >> >> ## Evaluation >> >> The root cause of this bug is that the D3D pipeline did not handle a return >> code of `D3DERR_DEVICEREMOVED` from `TestCooperativeLevel`. When that error >> occurs, an application needs to destroy and recreate the Direct3D device. >> >> The solution is to implement a new `D3DPipeline::reinitialize` method that >> will destroy the native D3D device and dispose the existing ResourceFactory >> objects and their associated BaseContext objects upon receiving >> `D3DERR_DEVICEREMOVED`. Note that the `D3DPipeline` Java object singleton is >> not recreated (it remains a singleton). In support of this, I implemented >> proper disposal logic in `BaseResourceFactory` and `BaseContext` to clean >> everything up and also to avoid memory leaks. >> >> Additionally, there were several places that assumed that some textures (and >> mesh vertices) could be made permanent and never need to handle the case of >> a lost device. These all had to be fixed to allow for the possibility of a >> lost device and associated resource factory. They included: >> >> * UploadingPainter and PresentingPainter need to set the resource factory to >> null when not ready, so it will get the (possibly new) factory the next time >> it tries. >> * The gradient texture cache in `PaintHelper` has to be cleared and >> recreated when the surface is lost >> * The 3D triangle mesh and Phong material classes need to be disposed when >> the resource factory is disposed. >> * WebView often renders to a texture image at a time other than from the >> main rendering job, so needs to directly handle the case of a resource >> factory that is lost. >> * Decora PPSRenderer assumed that the resource factory never went away; it >> also accessed it on the wrong thread. Both problems were addressed by >> deferring the initialization of the resource factory and handling the case >> where the device is disposed. >> * Snapshot needs to allow for the platform image to be null if the device >> has been disposed. >> >> ## Notes to Reviewers >> >> I created this PR from a branch that contains the original 4 commits by >> @Schmidor (rebased on top of the current `master`) and then a single commit >> on top of that to complete it. This allows anyone who is interested to >> easily see the diffs between this PR and Oliver's original Draft PR. Most >> reviewers can just go to the list of "Files" and see the aggregate diffs. >> >> During the course of my testing I discovered three outstanding problems, >> which will be handled by filing follow-up issues. Once I file them, I'll add >> a comment to this PR with the bug IDs. >> >> 1. Media: a media stream playing at the time of a reconnect doesn't continue >> playing. Reloading the media works fine. This is not directly related to >> this bug, since it also happens with the software pipeline. >> 2. Canvas: doesn't preserve the contents after a device reconnect (noticed >> while running Zoomy, where the BG color is wrong after device >> reinitialization). This might point to a need to let the app know they have >> to repaint, since there is no possible way to preserve the contents of the >> texture when the device is lost. >> 3. WebView: there is a possible memory leak when device isn't ready after >> first reset, due to a `WCRenderQueueImpl::gc` instance being held in a >> JNIGlobal. This looks like a preexisting condition that could happen with a >> page (re)load today. It happens rarely. >> >> This is a complicated enough change that I'd like three reviewers. The bulk >> of the changes are Windows-specific, but there are changes in common code so >> at least a sanity check needs to be done on all platforms using both the HW >> and SW pipelines. The case of a disposed device can currently only happen on >> Windows with the D3D pipeline. > > Kevin Rushforth has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments to fix typo in comment Fix looks good to me. Sanity testing with all three d3d, es2, sw pipelines behave as expected. Apps work as expected in case of remote desktop, sleep, screen lock. ------------- Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/430
