This Issue is describing a real problem found during a debugging session that could theoretically have consequences for apps, but we have not received any reports of rendering errors in the field because of it. There might be a slight performance penalty because of the underlying problem, but it has not been measured and it would be hard to estimate the potential gains without a fix. The visible consequences that kicked off the code reading that discovered the problem happened during debugging a PR that has not been merged (and the fix in that PR will be adjusted to account for this discrepancy before it is merged) and so these visible anomalies were never seen in the field.
Some background is that our frames are eventually reduced to a DisplayList which can contain any number of hierarchical DisplayList children nested in a tree structure. To reduce processing operations that are not visible, the DisplayList can be dispatched with a cull rect that will skip any operations that are currently not visible inside that cull rect. The results are consistent as long as the cull rect used for dispatching is the same.
The cause of the problem is that the first pass and rendering pass use different calculations to compute the cull rects used to Dispatch child DisplayLists and sometimes they produce different results which means the child DisplayLists are dispatched with a different subset of their operations on each pass. Their top level cull rects are the same, but when they reach child DisplayLists they compute a new cull rect for that child based on the information they've accumulated from the parent DisplayLists, including any outstanding transform and clip operations, the implicit clips from SaveLayer operations, and in some cases the size of a temporary buffer used while rendering pieces of the scene for later filtering.
The discrepancy was discovered while investigating a fix for data sharing conflicts in the TextFrame (#182539). In the PR, data representing the rendering of text is saved during the first pass for the second pass to re-use (annotated with bounds information). In practice it was discovered that some of the data saved by the first pass was not used/needed by the second pass. In investigating why this was happening, the cull rects used to dispatch child DisplayLists were compared and discovered to be different between the 2 passes (details below) - which means that the specific DisplayList Ops encountered during the 2 passes would be different. Thus, the first pass produced information for some drawText operations that were never actually rendered. The first implementation of the new mechanism was not expecting that discrepancy to occur. It will be adjusted to account for this.
For now, we've only discovered that the first pass is using cull rects that are larger (i.e. Rect::Contains()) than those used by the rendering pass (see the example list below), so for those cases investigated the first pass does examine a superset of operations compared to the rendering pass. In another example not captured, up to 15 drawText calls appeared (together in one set) and had to be ignored. This condition was encountered many times while scrolling the Wondrous app, though it hasn't shown any artifacts yet (until the first attempt at the new mechanism in the PR).
But, (the potentially scary part) we cannot prove that the first pass will never miss some operations that the rendering pass processes. If the first pass ever processes a subset of the operations compared to the rendering pass we might end up with bad output when the rendering operation does not find the precomputed data that it is expecting. Thankfully that wasn't observed while debugging that PR and if it happened then we might have expected to see more reports of problems as of yet.
The currently demonstrated case of the first pass using a superset of operations shouldn't cause any issues for the current code base other than the inefficiency of processing (and precomputing information for) rendering operations that may not actually be rendered.
For now, in working on the engine, first pass calculations should present their pre-computed data in a way that the rendering pass can match up the data even if the list of ops are not the same. They shouldn't be out of order, though, since dispatch culling can only omit operations, not change their order. Unless we can prove that the first pass never misses any operation, the rendering pass should strongly consider having a backup plan if it does not receive the precomputed data as well.
Log of cull rects encountered during a Wondrous frame that skipped a drawText call
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1347)] Render2Target first pass cull rect: ((0, 0) => (1080, 2400))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, 0) => (411.429, 914.286))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, 0) => (411.429, 313))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, 0) => (411.429, 914.286))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, -501.719) => (411.429, 412.567))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, 0) => (411.429, 914.286))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, 0) => (411.429, 914.286))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((0, -820.286) => (411.429, 94))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((-9.82143, -808.286) => (401.607, 106))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((-88.1786, -825.286) => (323.25, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((-166.536, -825.286) => (244.893, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((-244.893, -825.286) => (166.536, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1164)] FirstPass cull rect: ((-323.25, -825.286) => (88.1786, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(1369)] Render2Target render cull rect: ((0, 0) => (1080, 2400))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, 0) => (411.429, 914.286))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, 0) => (411.429, 313))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, 0) => (411.429, 820.19))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, -501.719) => (411.429, 318.471))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, 0) => (411.429, 820.19))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, 0) => (411.429, 820.19))
## the next line demonstrates that the rendering pass encountered data from the first pass that it doesn't need
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(863)] Skipped 1 RenderTextFrames
## the extra RenderTextFrame entry from the first pass was detected and skipped without repercurssions
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((0, -820.286) => (411.429, 94))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((-9.82143, -808.286) => (401.607, 106))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((-88.1786, -825.286) => (323.25, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((-166.536, -825.286) => (244.893, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((-244.893, -825.286) => (166.536, 89))
E/flutter (25172): [ERROR:flutter/impeller/display_list/dl_dispatcher.cc(825)] DispatcherBase cull rect: ((-323.25, -825.286) => (88.1786, 89))
This Issue is describing a real problem found during a debugging session that could theoretically have consequences for apps, but we have not received any reports of rendering errors in the field because of it. There might be a slight performance penalty because of the underlying problem, but it has not been measured and it would be hard to estimate the potential gains without a fix. The visible consequences that kicked off the code reading that discovered the problem happened during debugging a PR that has not been merged (and the fix in that PR will be adjusted to account for this discrepancy before it is merged) and so these visible anomalies were never seen in the field.
Some background is that our frames are eventually reduced to a DisplayList which can contain any number of hierarchical DisplayList children nested in a tree structure. To reduce processing operations that are not visible, the DisplayList can be dispatched with a cull rect that will skip any operations that are currently not visible inside that cull rect. The results are consistent as long as the cull rect used for dispatching is the same.
The cause of the problem is that the first pass and rendering pass use different calculations to compute the cull rects used to Dispatch child DisplayLists and sometimes they produce different results which means the child DisplayLists are dispatched with a different subset of their operations on each pass. Their top level cull rects are the same, but when they reach child DisplayLists they compute a new cull rect for that child based on the information they've accumulated from the parent DisplayLists, including any outstanding transform and clip operations, the implicit clips from SaveLayer operations, and in some cases the size of a temporary buffer used while rendering pieces of the scene for later filtering.
The discrepancy was discovered while investigating a fix for data sharing conflicts in the TextFrame (#182539). In the PR, data representing the rendering of text is saved during the first pass for the second pass to re-use (annotated with bounds information). In practice it was discovered that some of the data saved by the first pass was not used/needed by the second pass. In investigating why this was happening, the cull rects used to dispatch child DisplayLists were compared and discovered to be different between the 2 passes (details below) - which means that the specific DisplayList Ops encountered during the 2 passes would be different. Thus, the first pass produced information for some drawText operations that were never actually rendered. The first implementation of the new mechanism was not expecting that discrepancy to occur. It will be adjusted to account for this.
For now, we've only discovered that the first pass is using cull rects that are larger (i.e. Rect::Contains()) than those used by the rendering pass (see the example list below), so for those cases investigated the first pass does examine a superset of operations compared to the rendering pass. In another example not captured, up to 15 drawText calls appeared (together in one set) and had to be ignored. This condition was encountered many times while scrolling the Wondrous app, though it hasn't shown any artifacts yet (until the first attempt at the new mechanism in the PR).
But, (the potentially scary part) we cannot prove that the first pass will never miss some operations that the rendering pass processes. If the first pass ever processes a subset of the operations compared to the rendering pass we might end up with bad output when the rendering operation does not find the precomputed data that it is expecting. Thankfully that wasn't observed while debugging that PR and if it happened then we might have expected to see more reports of problems as of yet.
The currently demonstrated case of the first pass using a superset of operations shouldn't cause any issues for the current code base other than the inefficiency of processing (and precomputing information for) rendering operations that may not actually be rendered.
For now, in working on the engine, first pass calculations should present their pre-computed data in a way that the rendering pass can match up the data even if the list of ops are not the same. They shouldn't be out of order, though, since dispatch culling can only omit operations, not change their order. Unless we can prove that the first pass never misses any operation, the rendering pass should strongly consider having a backup plan if it does not receive the precomputed data as well.
Log of cull rects encountered during a Wondrous frame that skipped a drawText call