Remove `GLWrapper` and move implementation to `IRenderer` #5332

smoogipoo · 2022-08-03T06:13:27Z

Prereqs:

Internalise TextureGL and rewrite Texture to take its role #5330

For the most part, this is a 1-1 migration of GLWrapper implementation into OpenGLRenderer, with appropriate interfaces added to IRenderer. The rest is passing IRenderer/OpenGLRenderer to GL-specific classes (VBOs, Shaders, etc).

Important changes of note:

Drawable DIs in an IRenderer to dispose DrawNodes. Somehow, DrawNode disposal has to be enqueued to run 3 frames behind. There's several ways to do this, and I'm not sure if the way I've chosen here is the best. Alternatives:
1. Make the disposal queue static.
  - Reason not done: sounds unsafe.
2. Change Drawable.CreateDrawNode() to take an IRenderer parameter and construct DrawNodes with the renderer.
  - Reason not done: larger breaking change.
3. Store IRenderer from Drawable.Draw()/Drawable.DrawOpaqueInteriorSubTree().
  - Reason not done: I'm not sure if this will work if exceptions are thrown (e.g. BufferedDrawNode() calls base.Draw() very late.
TexturedShaderDrawNode interface has changed slightly because RequiresRoundedShader() needs an IRenderer parameter. osu!-side this only results in a changes to one file.
Size check removed from TextureUpload. In Internalise TextureGL and rewrite Texture to take its role #5330 I've moved the check to Texture.SetData().

vNext

`GLWrapper` removed, replaced by `IRenderer`

The static GLWrapper class no longer exists, and all drawing functions should be performed via the DrawNode.Draw() IRenderer parameter. There is a 1-1 compatibility between the classes.

`TexturedShaderDrawNode` interface changed slightly

Shader -> GetAppropriateShader(IRenderer).
RequriesRoundedShader -> RequiresRoundedShader(IRenderer)

smoogipoo · 2022-08-03T06:17:21Z

Will need a test on iOS, but I'm fairly confident it should work.

peppy · 2022-08-05T06:44:23Z

osu.Framework/Graphics/Rendering/IRenderer.cs

+
+        /// <summary>
+        /// The maximum number of pixels to upload per frame.
+        /// Defaults to 2 megapixels (8mb alloc).


Kinda weird to specify the defaults in the interface when it doesn't guarantee them, but I guess it's okay.

peppy · 2022-08-05T07:10:33Z

While testing on iOS, I got some crashes:

(from the finalizer thread)

and

System.NullReferenceException: Object reference not set to an instance of an object
  at osu.Framework.Graphics.OpenGL.Buffers.FrameBuffer+FrameBufferTexture.set_Width (System.Int32 value) [0x00000] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/OpenGL/Buffers/FrameBuffer.cs:136 
  at osu.Framework.Graphics.OpenGL.Textures.TextureGL..ctor (osu.Framework.Graphics.OpenGL.OpenGLRenderer renderer, System.Int32 width, System.Int32 height, System.Boolean manualMipmaps, osuTK.Graphics.ES30.All filteringMode, SixLabors.ImageSharp.PixelFormats.Rgba32 initialisationColour) [0x00025] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/OpenGL/Textures/TextureGL.cs:66 
  at osu.Framework.Graphics.OpenGL.Buffers.FrameBuffer+FrameBufferTexture..ctor (osu.Framework.Graphics.OpenGL.OpenGLRenderer renderer, osuTK.Graphics.ES30.All filteringMode) [0x00000] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/OpenGL/Buffers/FrameBuffer.cs:125 
  at osu.Framework.Graphics.OpenGL.Buffers.FrameBuffer..ctor (osu.Framework.Graphics.OpenGL.OpenGLRenderer renderer, osuTK.Graphics.ES30.RenderbufferInternalFormat[] renderBufferFormats, osuTK.Graphics.ES30.All filteringMode) [0x00030] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/OpenGL/Buffers/FrameBuffer.cs:27 
  at osu.Framework.Graphics.OpenGL.OpenGLRenderer.CreateFrameBuffer (osu.Framework.Graphics.Rendering.RenderBufferFormat[] renderBufferFormats, osu.Framework.Graphics.Textures.TextureFilteringMode filteringMode) [0x000a5] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/OpenGL/OpenGLRenderer.cs:855 
  at osu.Framework.Graphics.BufferedDrawNodeSharedData.Initialise (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x00021] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/BufferedDrawNodeSharedData.cs:92 
  at osu.Framework.Graphics.BufferedDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x00013] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/BufferedDrawNode.cs:87 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Graphics.Containers.CompositeDrawable+CompositeDrawableDrawNode.Draw (osu.Framework.Graphics.Rendering.IRenderer renderer) [0x0009d] in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs:204 
  at osu.Framework.Platform.GameHost.DrawFrame () [0x00168] in /Users/dean/Projects/osu-framework/osu.Framework/Platform/GameHost.cs:506 
  at osu.Framework.Threading.GameThread.processFrame () [0x000d9] in /Users/dean/Projects/osu-framework/osu.Framework/Threading/GameThread.cs:454 
--- End of stack trace from previous location where exception was thrown ---

  at osu.Framework.Platform.GameHost+<>c__DisplayClass133_0.<abortExecutionFromException>b__0 () [0x00002] in /Users/dean/Projects/osu-framework/osu.Framework/Platform/GameHost.cs:390 
  at osu.Framework.Threading.ScheduledDelegate.InvokeTask () [0x00010] in /Users/dean/Projects/osu-framework/osu.Framework/Threading/ScheduledDelegate.cs:106 
  at osu.Framework.Threading.ScheduledDelegate.RunTaskInternal () [0x0002b] in /Users/dean/Projects/osu-framework/osu.Framework/Threading/ScheduledDelegate.cs:92 
  at osu.Framework.Threading.Scheduler.Update () [0x0008a] in /Users/dean/Projects/osu-framework/osu.Framework/Threading/Scheduler.cs:125 
  at osu.Framework.Threading.GameThread.processFrame () [0x00172] in /Users/dean/Projects/osu-framework/osu.Framework/Threading/GameThread.cs:467 
  at osu.Framework.Threading.GameThread.RunSingleFrame () [0x00001] in /Users/dean/Projects/osu-framework/osu.Framework/Threading/GameThread.cs:296 
  at osu.Framework.Platform.ThreadRunner.RunMainLoop () [0x0005f] in /Users/dean/Projects/osu-framework/osu.Framework/Platform/ThreadRunner.cs:114 
  at osu.Framework.Platform.GameHost.windowUpdate () [0x00026] in /Users/dean/Projects/osu-framework/osu.Framework/Platform/GameHost.cs:850 
  at osu.Framework.Platform.GameHost.<Run>b__155_2 (System.Object _, osuTK.FrameEventArgs _) [0x00000] in /Users/dean/Projects/osu-framework/osu.Framework/Platform/GameHost.cs:754 
  at (wrapper delegate-invoke) System.EventHandler`1[osuTK.FrameEventArgs].invoke_void_object_TEventArgs(object,osuTK.FrameEventArgs)
  at osuTK.iOS.iOSGameView.OnUpdateFrame (osuTK.FrameEventArgs e) [0x0000a] in <0fc4a59399e2486f977f9fdb230650a4>:0 
  at osuTK.iOS.iOSGameView.RunIteration (Foundation.NSTimer timer) [0x00042] in <0fc4a59399e2486f977f9fdb230650a4>:0 
  at osuTK.iOS.CADisplayLinkTimeSource.RunIteration (Foundation.NSObject parameter) [0x00000] in <0fc4a59399e2486f977f9fdb230650a4>:0 
  at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr)
  at UIKit.UIApplication.Main (System.String[] args, System.Type principalClass, System.Type delegateClass) [0x0003b] in /Users/builder/azdo/_work/1/s/xamarin-macios/src/UIKit/UIApplication.cs:83 
  at osu.Framework.Tests.Application.Main (System.String[] args) [0x00001] in /Users/dean/Projects/osu-framework/osu.Framework.Tests.iOS/Application.cs:18

Both of these only started after selecting osu.Framework.Tests.Visual.Drawables.TestSceneCircularArcBoundingBox, and now occur on startup due to that test being loaded first.

smoogipoo · 2022-08-05T07:13:22Z

Good catch, came about due to the removal of the null check in the previous PR that wasn't tested in combination with this branch. Fixed.

peppy · 2022-08-05T07:14:26Z

I guess the first crash was just triggered as a side-effect of the second and in normal execution won't happen due to the Renderer being the first thing to be initialised now?

smoogipoo · 2022-08-05T07:15:24Z

That's correct, yeah.

smoogipoo added 5 commits August 3, 2022 14:20

Populate IRenderer/OpenGLRenderer with remaining GLWrapper methods

ae2a97e

Remove GLWrapper reference from iOS

90c1ae0

Entirely remove GLWrapper, update all references

16d1c2d

Internalise OpenGLRenderer

e6ab640

Remove GL-specific members from IRenderer

8dbdb47

smoogipoo added the area:graphics label Aug 3, 2022

pull-request-size bot added the size/XXL label Aug 3, 2022

Fix missing using

2f1a7a7

smoogipoo mentioned this pull request Aug 3, 2022

Apply IRenderer changes ppy/osu#19560

Merged

2 tasks

Merge branch 'master' into irenderer-glwrapper

b8e9eea

peppy reviewed Aug 5, 2022

View reviewed changes

Fix crash due to init order

634416d

peppy approved these changes Aug 5, 2022

View reviewed changes

peppy enabled auto-merge August 5, 2022 07:21

Merge branch 'master' into irenderer-glwrapper

75263ca

peppy merged commit 306ce5e into ppy:master Aug 5, 2022

smoogipoo deleted the irenderer-glwrapper branch September 11, 2023 02:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `GLWrapper` and move implementation to `IRenderer` #5332

Remove `GLWrapper` and move implementation to `IRenderer` #5332

smoogipoo commented Aug 3, 2022 •

edited by peppy

Loading

smoogipoo commented Aug 3, 2022

peppy Aug 5, 2022

peppy commented Aug 5, 2022 •

edited

Loading

smoogipoo commented Aug 5, 2022 •

edited

Loading

peppy commented Aug 5, 2022

smoogipoo commented Aug 5, 2022

Remove GLWrapper and move implementation to IRenderer #5332

Remove GLWrapper and move implementation to IRenderer #5332

Conversation

smoogipoo commented Aug 3, 2022 • edited by peppy Loading

vNext

GLWrapper removed, replaced by IRenderer

TexturedShaderDrawNode interface changed slightly

smoogipoo commented Aug 3, 2022

peppy Aug 5, 2022

Choose a reason for hiding this comment

peppy commented Aug 5, 2022 • edited Loading

smoogipoo commented Aug 5, 2022 • edited Loading

peppy commented Aug 5, 2022

smoogipoo commented Aug 5, 2022

Remove `GLWrapper` and move implementation to `IRenderer` #5332

Remove `GLWrapper` and move implementation to `IRenderer` #5332

smoogipoo commented Aug 3, 2022 •

edited by peppy

Loading

`GLWrapper` removed, replaced by `IRenderer`

`TexturedShaderDrawNode` interface changed slightly

peppy commented Aug 5, 2022 •

edited

Loading

smoogipoo commented Aug 5, 2022 •

edited

Loading