Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(android): GLCanvasElement #18216

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

feat(android): GLCanvasElement #18216

wants to merge 12 commits into from

Conversation

ramezgerges
Copy link
Contributor

@ramezgerges ramezgerges commented Sep 16, 2024 •
edited
Loading

GitHub Issue (If applicable): #9405, follows from #16621

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@github-actions github-actions bot added platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform area/automation Categorizes an issue or PR as relevant to project automation labels Sep 16, 2024
@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-18216/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-18216/index.html

jeromelaban
jeromelaban reviewed Oct 2, 2024
View reviewed changes
src/Uno.UI/UI/Xaml/Controls/Border/BorderLayerRenderer.Android.cs
Comment on lines -46 to -49
if (newState.Equals(previousLayoutState))
{
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a performance issue. Do you recall why you needed to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's simply very incorrect. The fact that the previous and new layout states are the same doesn't mean that what will actually be drawn is the same. I looked quickly for a better condition but it didn't work out. I can look into optimizing this in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I managed to create a repro that's completely independent of GLCanvasElement to illustrate the problem and opened a separate PR.
#18408

@ramezgerges ramezgerges mentioned this pull request Oct 3, 2024
Merged
6 tasks
@ramezgerges
chore: import targetframework-override.props in Graphics3DGL.csproj a…
c314461 
…nd remove NetPrevious-android from TargetFrameworks
ramezgerges added a commit to ramezgerges/uno that referenced this pull request Oct 4, 2024
@ramezgerges
chore: get important changes from unoplatform#18216
83a3ff0
@jeromelaban
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-18216/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-18216/index.html

ramezgerges added a commit to ramezgerges/uno that referenced this pull request Oct 7, 2024
@ramezgerges
chore: get important changes from unoplatform#18216
fbe830a
@jeromelaban
Copy link
Member

@ramezgerges

Section changed: SearchContent
fail: Microsoft.UI.Xaml.UIElement[0]
      OnElementUnloaded failed in FrameworkElement
      System.NullReferenceException: Object reference not set to an instance of an object.
         at UITests.Shared.Windows_UI_Composition.RotatingCubeGlCanvasElement.OnDestroy(GL Gl) in C:\a\1\s\src\SamplesApp\UITests.Shared\Windows_UI_Composition\RotatingCubeGlCanvasElement.cs:line 121
         at Uno.WinUI.Graphics3DGL.GLCanvasElement.OnUnloaded(Object sender, RoutedEventArgs routedEventArgs) in C:\a\1\s\src\AddIns\Uno.WinUI.Graphics3DGL\GLCanvasElement.cs:line 286
         at Microsoft.UI.Xaml.FrameworkElement.<OnFwEltUnloaded>g__InvokeUnloaded|86_0() in C:\a\1\s\src\Uno.UI\UI\Xaml\FrameworkElement.crossruntime.cs:line 206
         at Microsoft.UI.Xaml.FrameworkElement.<OnFwEltUnloaded>g__InvokeUnloadedWithTry|86_1() in C:\a\1\s\src\Uno.UI\UI\Xaml\FrameworkElement.crossruntime.cs:line 221
info: Uno.UI.Runtime.Skia.MacOS.MacOSNativeOpenGLWrapper[0]
      Found EGL version 1.5.
fail: Microsoft.UI.Xaml.UIElement[0]
      OnElementLoaded failed in FrameworkElement
      System.Exception: Vertex shader failed to compile: ERROR: 0:2: '#' : invalid character
      ERROR: 0:2: 'for' : syntax error

         at UITests.Shared.Windows_UI_Composition.SimpleTriangleGlCanvasElement.Init(GL gl) in C:\a\1\s\src\SamplesApp\UITests.Shared\Windows_UI_Composition\SimpleTriangleGlCanvasElement.cs:line 80
         at Uno.WinUI.Graphics3DGL.GLCanvasElement.OnLoaded(Object sender, RoutedEventArgs routedEventArgs) in C:\a\1\s\src\AddIns\Uno.WinUI.Graphics3DGL\GLCanvasElement.cs:line 245
         at Microsoft.UI.Xaml.FrameworkElement.<OnFwEltLoaded>g__InvokeLoadedWithTry|327_1() in C:\a\1\s\src\Uno.UI\UI\Xaml\FrameworkElement.Layout.crossruntime.cs:line 111
dbug: Uno.UI.Runtime.Skia.MacOS.MacOSWindowHost[0]
      SetPointerCapture not supported on macOS.
dbug: Uno.UI.Runtime.Skia.MacOS.MacOSWindowHost[0]
      ReleasePointerCapture not supported on macOS.
Section changed: SearchContent
info: Uno.UI.Runtime.Skia.MacOS.MacOSNativeOpenGLWrapper[0]
      Found EGL version 1.5.
fail: Microsoft.UI.Xaml.UIElement[0]
      OnElementLoaded failed in FrameworkElement
      System.Exception: Error compiling shader of type VertexShader, failed with error ERROR: 0:2: '#' : invalid character
      ERROR: 0:2: 'for' : syntax error

         at UITests.Shared.Windows_UI_Composition.RotatingCubeGlCanvasElement.Shader.LoadShader(ShaderType type, String src) in C:\a\1\s\src\SamplesApp\UITests.Shared\Windows_UI_Composition\RotatingCubeGlCanvasElement.cs:line 241
         at UITests.Shared.Windows_UI_Composition.RotatingCubeGlCanvasElement.Shader..ctor(GL gl, String vertexShaderSource, String fragmentShaderSource) in C:\a\1\s\src\SamplesApp\UITests.Shared\Windows_UI_Composition\RotatingCubeGlCanvasElement.cs:line 175
         at UITests.Shared.Windows_UI_Composition.RotatingCubeGlCanvasElement.Init(GL Gl) in C:\a\1\s\src\SamplesApp\UITests.Shared\Windows_UI_Composition\RotatingCubeGlCanvasElement.cs:line 113
         at Uno.WinUI.Graphics3DGL.GLCanvasElement.OnLoaded(Object sender, RoutedEventArgs routedEventArgs) in C:\a\1\s\src\AddIns\Uno.WinUI.Graphics3DGL\GLCanvasElement.cs:line 245
         at Microsoft.UI.Xaml.FrameworkElement.<OnFwEltLoaded>g__InvokeLoadedWithTry|327_1() in C:\a\1\s\src\Uno.UI\UI\Xaml\FrameworkElement.Layout.crossruntime.cs:line 111

@ramezgerges ramezgerges mentioned this pull request Oct 8, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeromelaban jeromelaban jeromelaban left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area/automation Categorizes an issue or PR as relevant to project automation kind/documentation platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ramezgerges @unodevops @jeromelaban @Youssef1313