Text distorted in presence of two platform views #46911
Assignees
Labels
Milestone
Comments
kf6gpe
added
engine
|
This looks somewhat similar to previous multitexturing font issues we'd seen on iOS devices (and still do in some cases on iPhone 4S and 5). Similar previous issues for ref: cc'ed @brianosman on the internal issue. |
|
This currently seems to be an upstream Skia issue so we do not expect to be directly working on it ourselves. Leaving open for tracking purposes so that we can check on it occasionally to see if progress is being made. |
|
We believe this is an upstream Skia issue related to multi-texturing. This is back on our plate to verify whether disabling multi-texturing eliminates the issue and to get back to the Skia team with the answer. |
|
Just got back from vacation and working through a backlog. The root cause has not been identified yet and the multitexturing hypothesis needs to be verified first by setting the |
csells
changed the title
|
Workaround on internal thread does not work for all cases; this is blocking our customer. I raised the priority. |
|
As of today, Jim on the Skia team is working on this. |
|
The cause has been identified as GL context isolation. Across all the bug trackers I was pinged on, this seems to have highest priority. Working on mitigation for this now. |
|
Fixed after reverting texture bindings in the embedder. cl/291239009 |
|
Reopened internally. The current course of action is described in my comment in the internal tracker that I am pasting here in its entirety.
|
|
We believe this is the embedder stomping on the Skia GL context. Removing the skia label for triage. |
chinmaygarde
added a commit
to chinmaygarde/flutter_engine
that referenced
this issue
Feb 17, 2020
…nGL context in composition callbacks. During the implementation of custom compositor integration, the embedder gets callbacks on the render thread to prepare render targets (framebuffers, textures, etc) for the engine to render into, callbacks to present these render targets along with platform managed contents, and, callbacks to collect render targets once they can no longer be recycled by the engine in subsequent frames. During these callbacks, the engine mandates the OpenGL state on the render thread be preserved. This restriction has been the source of hard to isolate issues where the embedder trampled on the OpenGL bindings state in the callback but failed to restore state before control went back to the engine. Due to the nature of the OpenGL API, such errors are easy to make and overlook. This patch lifts the restriction from the embedder. Embedders may now freely work with the OpenGL state in custom compositor callbacks and the engine will make sure to disregard OpenGL bindings when control flows back to it. Disregarding current OpenGL state has a certain performance penalty and the majority of this patch handles refactoring various engine embedder components such that this happens only once per frame. The most trivial version of this patch would reset context bindings on every transition of control flow from the embedder to the engine. However, that naive approach would have necessitated more than 50 binding resets in existing unit-test cases (depending on the number of platform view interleaving levels and render target recycling hit rates). In this implementation, bindings will be reset only once per frame and this does not depend on the number of platform views in the scene. The majority of this patch is a refactoring of engine subsystems used in `ExternalViewEmbedder::SubmitFrame` which is thoroughly documented with each opportunity for the embedder to invalidate OpenGL state tagged. The refactoring also enables the implementation of the following optimizations to engine behavior which should aid in reducing the memory needed for the creation of render targets. These optimization include: * The engine will only ask the embedder for render targets in which it expects to render into. This was a quirk in the way in which root and non-root render targets were handled. The engine could require the embedder to create a render target but then realize it didn’t have anything to render into it. In the presentation callback, it would skip that render target. But the embedder still had to allocate that extra render target. This will no longer be the case and should reduce memory use. * The engine may now skip always realizing (via the embedder render target creation callback) and presenting the root render target. This was also a side effect of the same quirk. Previously, the engine would always ask the embedder to present the root render target even if it was empty. Since this is no longer the case, few render targets should be allocated which will reduce memory consumption. * The engine will now ask the embedder to collect unused render targets before it asks it to create new ones. The previous behavior was to ask the embedder for new targets and then collect old ones. This would cause spikes in memory use when the size of the render targets would change. These memory use spikes should now be troughs. * The previous render target cache also considered the platform view ID in cache viability considerations (instead of just the size of the render target). This was a bug which has been fixed. This should lead to better cache utilization in some situations. These optimizations are now codified in unit-tests and the updated test expectations are a result of these optimizations now being in place. Fixes flutter/flutter#50751 Fixes flutter/flutter#46911 Fixes flutter/flutter#43778 Fixes https://buganizer.corp.google.com/issues/146142979
|
After flutter/engine#16653, the embedder should no longer need to restore OpenGL state before transferring control back to the engine. |
|
Will close this issue once the patch lands. There are no pending Flutter deliverables on this front. |
chinmaygarde
added a commit
to flutter/engine
that referenced
this issue
Feb 18, 2020
…nGL context in composition callbacks. (#16653) During the implementation of custom compositor integration, the embedder gets callbacks on the render thread to prepare render targets (framebuffers, textures, etc) for the engine to render into, callbacks to present these render targets along with platform managed contents, and, callbacks to collect render targets once they can no longer be recycled by the engine in subsequent frames. During these callbacks, the engine mandates the OpenGL state on the render thread be preserved. This restriction has been the source of hard to isolate issues where the embedder trampled on the OpenGL bindings state in the callback but failed to restore state before control went back to the engine. Due to the nature of the OpenGL API, such errors are easy to make and overlook. This patch lifts the restriction from the embedder. Embedders may now freely work with the OpenGL state in custom compositor callbacks and the engine will make sure to disregard OpenGL bindings when control flows back to it. Disregarding current OpenGL state has a certain performance penalty and the majority of this patch handles refactoring various engine embedder components such that this happens only once per frame. The most trivial version of this patch would reset context bindings on every transition of control flow from the embedder to the engine. However, that naive approach would have necessitated more than 50 binding resets in existing unit-test cases (depending on the number of platform view interleaving levels and render target recycling hit rates). In this implementation, bindings will be reset only once per frame and this does not depend on the number of platform views in the scene. The majority of this patch is a refactoring of engine subsystems used in `ExternalViewEmbedder::SubmitFrame` which is thoroughly documented with each opportunity for the embedder to invalidate OpenGL state tagged. The refactoring also enables the implementation of the following optimizations to engine behavior which should aid in reducing the memory needed for the creation of render targets. These optimization include: * The engine will only ask the embedder for render targets in which it expects to render into. This was a quirk in the way in which root and non-root render targets were handled. The engine could require the embedder to create a render target but then realize it didn’t have anything to render into it. In the presentation callback, it would skip that render target. But the embedder still had to allocate that extra render target. This will no longer be the case and should reduce memory use. * The engine may now skip always realizing (via the embedder render target creation callback) and presenting the root render target. This was also a side effect of the same quirk. Previously, the engine would always ask the embedder to present the root render target even if it was empty. Since this is no longer the case, few render targets should be allocated which will reduce memory consumption. * The engine will now ask the embedder to collect unused render targets before it asks it to create new ones. The previous behavior was to ask the embedder for new targets and then collect old ones. This would cause spikes in memory use when the size of the render targets would change. These memory use spikes should now be troughs. * The previous render target cache also considered the platform view ID in cache viability considerations (instead of just the size of the render target). This was a bug which has been fixed. This should lead to better cache utilization in some situations. These optimizations are now codified in unit-tests and the updated test expectations are a result of these optimizations now being in place. * Fixes flutter/flutter#50751 * Fixes flutter/flutter#46911 * Fixes flutter/flutter#43778 * Fixes b/146142979
NoamDev
pushed a commit
to NoamDev/engine
that referenced
this issue
Feb 27, 2020
…nGL context in composition callbacks. (flutter#16653) During the implementation of custom compositor integration, the embedder gets callbacks on the render thread to prepare render targets (framebuffers, textures, etc) for the engine to render into, callbacks to present these render targets along with platform managed contents, and, callbacks to collect render targets once they can no longer be recycled by the engine in subsequent frames. During these callbacks, the engine mandates the OpenGL state on the render thread be preserved. This restriction has been the source of hard to isolate issues where the embedder trampled on the OpenGL bindings state in the callback but failed to restore state before control went back to the engine. Due to the nature of the OpenGL API, such errors are easy to make and overlook. This patch lifts the restriction from the embedder. Embedders may now freely work with the OpenGL state in custom compositor callbacks and the engine will make sure to disregard OpenGL bindings when control flows back to it. Disregarding current OpenGL state has a certain performance penalty and the majority of this patch handles refactoring various engine embedder components such that this happens only once per frame. The most trivial version of this patch would reset context bindings on every transition of control flow from the embedder to the engine. However, that naive approach would have necessitated more than 50 binding resets in existing unit-test cases (depending on the number of platform view interleaving levels and render target recycling hit rates). In this implementation, bindings will be reset only once per frame and this does not depend on the number of platform views in the scene. The majority of this patch is a refactoring of engine subsystems used in `ExternalViewEmbedder::SubmitFrame` which is thoroughly documented with each opportunity for the embedder to invalidate OpenGL state tagged. The refactoring also enables the implementation of the following optimizations to engine behavior which should aid in reducing the memory needed for the creation of render targets. These optimization include: * The engine will only ask the embedder for render targets in which it expects to render into. This was a quirk in the way in which root and non-root render targets were handled. The engine could require the embedder to create a render target but then realize it didn’t have anything to render into it. In the presentation callback, it would skip that render target. But the embedder still had to allocate that extra render target. This will no longer be the case and should reduce memory use. * The engine may now skip always realizing (via the embedder render target creation callback) and presenting the root render target. This was also a side effect of the same quirk. Previously, the engine would always ask the embedder to present the root render target even if it was empty. Since this is no longer the case, few render targets should be allocated which will reduce memory consumption. * The engine will now ask the embedder to collect unused render targets before it asks it to create new ones. The previous behavior was to ask the embedder for new targets and then collect old ones. This would cause spikes in memory use when the size of the render targets would change. These memory use spikes should now be troughs. * The previous render target cache also considered the platform view ID in cache viability considerations (instead of just the size of the render target). This was a bug which has been fixed. This should lead to better cache utilization in some situations. These optimizations are now codified in unit-tests and the updated test expectations are a result of these optimizations now being in place. * Fixes flutter/flutter#50751 * Fixes flutter/flutter#46911 * Fixes flutter/flutter#43778 * Fixes b/146142979
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
https://buganizer.corp.google.com/issues/146142979. cc @chinmaygarde and @cbracken.
The text was updated successfully, but these errors were encountered: