[WIP] Introduced visuals that render themselves on the render thread #2185

kekekeks · 2018-12-18T10:31:28Z

This is needed for displaying things like video output, animated images, foreign content from WritableBitmap, etc. The common thing about these usage scenarios is that they don't need the UI thread, but need a steady frame rate.
Since render thread timer ticks independently from UI thread the "render time critical" visuals can update themselves even if UI thread is completely blocked by the user code.

The API looks like this:

    public interface IRenderTimeCriticalVisual
    {
        bool HasNewFrame { get; }
        void ThreadSafeRender(DrawingContext context, Size logicalSize, double scaling);
    }

They are forced to have a separate layer since they are supposed to update very frequently.

This is DeferredRenderer implementation. For ImmediateRenderer I'm planning to connect it to the RenderLoop's Update and trigger Invalidate calls to the OS if the last frame had IRenderTimeCriticalVisual anywhere in the graph.

…Renderer support so far

…hat don't need the time feature all the time.

grokys

Looks good in general I think, though I'm concerned about it bit-rotting with a) no unit tests and b) no feature in the codebase that actually uses it. Is there a feature you plan on adding soon which makes use of this?

grokys · 2018-12-19T13:55:31Z

samples/ControlCatalog/MainView.xaml

@@ -37,6 +37,7 @@
      <TabItem Header="TreeView"><pages:TreeViewPage/></TabItem>
      <TabItem Header="TabControl"><pages:TabControlPage/></TabItem>
      <TabItem Header="Viewbox"><pages:ViewboxPage/></TabItem>
+      <TabItem Header="Time critical render"><pages:TimeCriticalRenderPage/></TabItem>


I think this might be more at home in RenderDemo as it's not a control as such.

grokys · 2018-12-19T13:57:13Z

src/Avalonia.Visuals/Media/DrawingContext.cs

        private int _currentLevel;


-        static readonly Stack<Stack<PushedState>> StateStackPool = new Stack<Stack<PushedState>>();
-        static readonly Stack<Stack<TransformContainer>> TransformStackPool = new Stack<Stack<TransformContainer>>();
+        private static readonly ThreadSafeObjectPool<Stack<PushedState>> StateStackPool =


Nit: these fields are capitalized as properties.

grokys · 2018-12-19T13:58:33Z

src/Avalonia.Visuals/Rendering/DeferredRenderer.cs

-
-                // Just return scene, layers weren't updated
+
+                var hasCriticalRenderTimeVisualUpdates = Layers.Any(l =>


Might not be the best idea to use LINQ in the rendering code?

grokys · 2018-12-19T14:06:32Z

src/Avalonia.Visuals/Media/DrawingContext.cs

-        private Stack<TransformContainer> _transformContainers = TransformStackPool.Count == 0
-            ? new Stack<TransformContainer>()
-            : TransformStackPool.Pop();
+        private Stack<PushedState> _states = StateStackPool.Get();


So this PR also refactors the pooling for the PushedState stacks? Is this required by this feature or is it independent of the feature?

Got a crash caused by the lack of locking in this code. Incorporated in the PR as a separate commit.

grokys · 2018-12-19T14:12:48Z

src/Avalonia.Visuals/Rendering/SceneGraph/IVisualNode.cs

+        /// <summary>
+        /// Gets the original visual size before transformations
+        /// </summary>
+        Size VisualSize { get; }


As far as I can tell, this is only needed by IRenderTimeCriticalVisual, is that right? Could this property be added to that interface? If it really needs to be added here it might be best to call it "PreTransformSize" or something?

It needs to be saved somewhere during the scene build pass.
The actual visual dimensions could be already changed on UI thread by the time we are trying to consume it.

grokys · 2018-12-19T14:20:47Z

src/Avalonia.Visuals/Visual.cs

@@ -239,6 +248,15 @@ protected IAvaloniaList<IVisual> VisualChildren
            private set;
        }

+        /// <summary>
+        /// Allows to explicitly force a rendering layer for a particular visual. Use with caution.


A few things here:

What does "Wants" mean here? Does it mean the request might not be respected? Because "wants" to me suggests a request that can be denied. If it will always be respected, maybe something like "ForceLayer" might be better?

This is diverging from WPF/UWP's way of requesting a new layer. Are we OK with that?

I think "Use with caution" should be in the <Remarks> section along with a description of why it should be used with caution.

Does it mean the request might not be respected

It's not respected by ImmediateRenderer. The idea is that it's just a hint.

I think we could remove this change since it's not really needed for the purposes of this PR.

grokys · 2018-12-19T14:21:17Z

src/Avalonia.Visuals/VisualTree/IRenderTimeCriticalVisual.cs

+        /// Checks if visual has time critical content. You need to call InvalidateVisual for this property to be re-queried.
+        /// </summary>
+        bool HasRenderTimeCriticalContent { get; }
+        /// <summary>


Nit: no newlines between properties

grokys · 2018-12-19T14:22:48Z

src/Windows/Avalonia.Win32/WindowImpl.cs

@@ -148,7 +148,9 @@ public IRenderer CreateRenderer(IRenderRoot root)
            if (customRendererFactory != null)
                return customRendererFactory.Create(root, loop);

-            return Win32Platform.UseDeferredRendering ? (IRenderer)new DeferredRenderer(root, loop) : new ImmediateRenderer(root);
+            return Win32Platform.UseDeferredRendering
+                ? (IRenderer)new DeferredRenderer(root, loop)


Nit: the ? in these sort of statements is usually put at the end of the condition for multi-line statements.

condition ? left : right;

is a pretty common pattern for multi-line expressions.

kekekeks · 2018-12-19T15:34:29Z

Is there a feature you plan on adding soon which makes use of this?

@jmacato is currently working on GIF animations support and we are planning to run them on the render thread since they obviously don't need anything from the UI thread to continue ticking.

kekekeks · 2018-12-19T15:36:14Z

Actually now that I think about it, we might want to let "render time critical" visuals not occupy a separate layer if they are opaque. Might want to change the code for that, since it would be way too expensive for each GIF to have a separate layer.

jmacato · 2018-12-20T08:31:55Z

@kekekeks i agree, but lets retain the ability to explicitly request for a render layer if it's possible

kekekeks · 2018-12-20T08:40:18Z

I think we need to introduce IsOpaque property. That's how Cocoa does it.

Gillibald · 2018-12-20T08:56:25Z

Isn't it possible to request a layer that is rendered fast enough to process such time critical scenarios? Avoiding a special interface would be ideal. Maybe i don't understand the real performance issue here. Isn't this approach just avoiding the heavy layouting etc by having it's own layer? If they are processed fom top to bottom this should still be fast enough. Someone could even request a layer that is smaller than the screen dimensions.

kekekeks · 2018-12-20T09:10:35Z

@Gillibald we aren't alone on the UI thread. It can be also busy processing user's business logic. Our code can also be quite expensive. So we can't get a steady frame rate unless render thread doesn't need anything from the UI thread. The only way to do so is to allow visuals to render themselves on the render thread bypassing our scene builder entirely.

This PR introduces an interface that allows visuals to do painting on the render thread. The current implementation always creates a layer because we can't properly track rendering operations, so we don't know what we need to invalidate and re-render. If such visual is opaque, we could skip layer creation and just re-render everything on top of the visual's rect.

Gillibald · 2018-12-20T10:59:39Z

So ThreadSafeRender is called every RenderLoop tick on the render thread as long as the HasNewFrame is true.

Why is ThreadSafeRender needed? Can't we use IVisual.Render

kekekeks · 2018-12-20T11:13:26Z

Why is ThreadSafeRender needed? Can't we use IVisual.Render

Visual.Bounds is a dependency property which can't be read from the render thread. The value of Visual.Bounds might also contain a new value that doesn't match the current scene. That's why there is a separate method. It also explicitly indicates in its name that method should be thread-safe.

Gillibald · 2018-12-20T12:16:40Z

Are there any real world examples that need the ability to issue render commands threw a DrawingContext? Isn't a special Bitmap implementation enough for this use case? In general someone wants a surface he can write to in a performant way or am I wrong?

jmacato · 2018-12-20T12:18:55Z

@Gillibald my gif renderer is one of those use cases, sometimes one needs a much more flexible approach when it comes to rendering, besides if we ever get to have 3d matrix transforms then it'll be easier to develop them with the current API than just having a fast bitmap control

Gillibald · 2018-12-20T13:39:22Z

@jmacato So you use draw calls other than DrawImage(Bitmap)?

jmacato · 2018-12-20T13:41:17Z

@Gillibald not yet, but i do have some valid use case for it later on like text overlay, etc. In any case, exposing the drawcontext is better than locking ourselves up later on especially when we lockdown the API surface @ v1.0

Gillibald · 2018-12-20T13:46:10Z

I just thought it would make sense to enable this kind of rendering just for Bitmaps so you could use all kinds of Bitmaps. Even RenderTargetBitmap. In the future you could have a 3DImage etc that kind of works the same. Everything just has to have a Bitmap implementation. Maybe I am wrong.

jmacato · 2018-12-20T13:47:30Z

i think this enables that though, they are not mutually-exclusive anyway :) if one needs more flexible rendering control they can use this, if they just want to push pixels fast enough then a IRenderTimeCriticalVisual based RTB/WB is good enough i think

ahopper · 2018-12-20T14:45:40Z

I certainly have a use case for more than just bitmaps as I have a changing grid and scales over various live graph/waterfall type displays and it would be nice to use the avalonia tools to draw these. They could be done in an overlay on the ui thread but this could look jumpy at times.

na2axl · 2018-12-20T17:12:35Z

I just thought it would make sense to enable this kind of rendering just for Bitmaps so you could use all kinds of Bitmaps.

I think that locking the use of this feature only for bitmaps is not a good idea, because it's impossible to know why the user want this feature and how he plan to use it. For example I use this feature for a game engine to write bitmaps generated by each game render passes and text for FPS information, logs, events, etc...

It could be heavy for the user to compute everything as a bitmap before passing it to the ThreadSafeRender method if he want to write more than a simple bitmap.

Another use case could be that the user want to create a custom frame by frame animation, a bouncing ball for example, with the set of DrawXXX methods from the DrawingContext.

Gillibald · 2018-12-20T17:18:46Z

Every layer in Avalonia is a RenderTargetBitmap so it will that way anyways. If someone wanted to issue draw commands he could use a RenderTargetBitmap. WritableBitmap and RenderTargetBitmap could implement that new logic by default. Is there any downside to this? Wouldn't WritableBitmap etc benefit from this?

new DrawingContext(RenderTargetBitmap bitmap)

That is just my opinion and wanted to discuss this a bit.

na2axl · 2018-12-20T17:50:53Z

Wouldn't WritableBitmap etc benefit from this?

Yes it's true, so it's to the user to know exactly why this feature have to be used.

Maybe for performances issues, or because he want a control on the frame rate of what he want to draw, to bypass the default 60 FPS of the render loop.

I think that we are not able to say that this feature will always be used for bitmaps, so to enable a larger set of use cases, it's better to let the user decide...

kekekeks · 2018-12-21T06:01:29Z

Every layer in Avalonia is a RenderTargetBitmap so it will that way anyways.

As I've said, if the visual is opaque, we don't need a separate layer. I'll do this optimization later.

ahopper · 2018-12-21T20:39:23Z

There appears to be an issue using FillRectangle with a brush created like so
SolidColorBrush filterBrush = new SolidColorBrush(Colors.White, 0.2);
This

Avalonia/src/Skia/Avalonia.Skia/DrawingContextImpl.cs

Line 495 in 04745d8

double opacity = brush.Opacity * _currentOpacity;

throws a 'Call from invalid thread' exception

Gillibald · 2018-12-21T20:42:53Z

You need make the brush immutable as far as I understand.

ahopper · 2018-12-21T20:50:14Z

Yep, using IBrush filterBrush = new SolidColorBrush(Colors.White, 0.2).ToImmutable();
works perfectly, thanks.

kekekeks · 2018-12-21T21:12:22Z

@ahopper You can't use types derived from AvaloniaObject on UI thread. Immutable non-visual brushes should be fine.

ahopper · 2018-12-24T18:38:56Z

This appears to write over the top of all other controls (except popups).

Gillibald · 2019-01-03T13:29:27Z

@ahopper That's because this solution has its own layer on top of the root layer. Popup should have its own window(root layer) on top of the parent window.

kekekeks · 2019-01-03T16:31:40Z

It seems to be an issue with layer ordering, I'll try to repro it with regular layers.

na2axl · 2019-02-07T17:04:02Z

No evolution on this WIP? Since the PR is pretty old, it seems that dotnet is unable to resolve the last build of this PR, and fallback to another build in which this feature is not available...

MarchingCube · 2019-03-12T17:35:19Z

@grokys @kekekeks It's been a while but I will repeat the question - any changes on the side of this PR? Do you need some help here? Could be useful for my purposes quite soon.

kekekeks · 2019-03-12T19:19:00Z

It's blocked by #2244

That bug causes "critical time" visuals to render themselves over everything else.

realvictorprm · 2019-12-29T14:31:32Z

Any updates? 🤔

danwalmsley · 2020-07-05T21:08:47Z

closing due to inactivity.

kekekeks added 4 commits December 18, 2018 12:54

Added WantsLayer property

0909407

Fixed some distortions on GL backend

bd4579f

More thread safety and ownsImpl argument for DrawingContext

23c40e1

Introduced the consept of render-time critical visuals. Only Deferred…

d7d1edf

…Renderer support so far

kekekeks requested a review from grokys December 18, 2018 10:31

kekekeks mentioned this pull request Dec 18, 2018

How to create a custom rendering control ? #2176

Closed

kekekeks added 3 commits December 18, 2018 18:30

Preserve LayoutBounds on visual node clone

e426a27

Preserve Visual's size in render node and pass it to ThreadSafeRender

31d2e34

Added support for critical render time visuals to ImmediateRenderer

758eaa4

kekekeks changed the title ~~[WIP] Introduced visuals that render themselves on the render thread~~ Introduced visuals that render themselves on the render thread Dec 18, 2018

kekekeks added 5 commits December 18, 2018 18:49

Actually ask render time critical visuals that they have a new frame

d1d4561

Fixed mock for render root

c5dee77

_scene might be null if first call to Render happens on render thread

2e6c819

Renamed HasNewFrame and added RenderTimeCriticalContent for visuals t…

319e99d

…hat don't need the time feature all the time.

Docs for IRenderTimeCriticalVisual

0608118

grokys requested changes Dec 19, 2018

View reviewed changes

kekekeks changed the title ~~Introduced visuals that render themselves on the render thread~~ [WIP] Introduced visuals that render themselves on the render thread Jan 9, 2019

jmacato mentioned this pull request Mar 2, 2019

Disposing Lock on WriteableBitmap fails to update the image. #1995

Closed

MarchingCube mentioned this pull request Mar 13, 2019

Allow for modifying Skia PlatformRenderInterface behavior. #2364

Merged

3 tasks

jmacato mentioned this pull request Apr 30, 2019

Support for Animated GIF and SVG #2481

Open

jmacato mentioned this pull request May 30, 2019

Video Player #2571

Closed

danwalmsley closed this Jul 5, 2020


		// Just return scene, layers weren't updated

		var hasCriticalRenderTimeVisualUpdates = Layers.Any(l =>

[WIP] Introduced visuals that render themselves on the render thread #2185

[WIP] Introduced visuals that render themselves on the render thread #2185

Conversation

kekekeks commented Dec 18, 2018 • edited Loading

grokys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grokys Dec 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks commented Dec 19, 2018

kekekeks commented Dec 19, 2018 • edited Loading

jmacato commented Dec 20, 2018

kekekeks commented Dec 20, 2018 • edited Loading

Gillibald commented Dec 20, 2018

kekekeks commented Dec 20, 2018

Gillibald commented Dec 20, 2018

kekekeks commented Dec 20, 2018 • edited Loading

Gillibald commented Dec 20, 2018

jmacato commented Dec 20, 2018

Gillibald commented Dec 20, 2018

jmacato commented Dec 20, 2018 • edited Loading

Gillibald commented Dec 20, 2018

jmacato commented Dec 20, 2018 • edited Loading

ahopper commented Dec 20, 2018

na2axl commented Dec 20, 2018

Gillibald commented Dec 20, 2018 • edited Loading

na2axl commented Dec 20, 2018

kekekeks commented Dec 21, 2018

ahopper commented Dec 21, 2018

Gillibald commented Dec 21, 2018

ahopper commented Dec 21, 2018

kekekeks commented Dec 21, 2018

ahopper commented Dec 24, 2018

Gillibald commented Jan 3, 2019

kekekeks commented Jan 3, 2019

na2axl commented Feb 7, 2019

MarchingCube commented Mar 12, 2019 • edited Loading

kekekeks commented Mar 12, 2019

realvictorprm commented Dec 29, 2019

danwalmsley commented Jul 5, 2020

kekekeks commented Dec 18, 2018 •

edited

Loading

grokys Dec 19, 2018 •

edited

Loading

kekekeks commented Dec 19, 2018 •

edited

Loading

kekekeks commented Dec 20, 2018 •

edited

Loading

kekekeks commented Dec 20, 2018 •

edited

Loading

jmacato commented Dec 20, 2018 •

edited

Loading

jmacato commented Dec 20, 2018 •

edited

Loading

Gillibald commented Dec 20, 2018 •

edited

Loading

MarchingCube commented Mar 12, 2019 •

edited

Loading