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.

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

Implement Blob component #5485

Merged
merged 7 commits into from
Oct 27, 2022
Merged

Implement Blob component #5485

merged 7 commits into from
Oct 27, 2022

Conversation

EVAST9919
Copy link
Contributor

@EVAST9919 EVAST9919 commented Oct 27, 2022

DrawNode logic is very similar to CircularProgress.
In the shader pointCount and search range may be probably adjusted further to improve performance even more but visuals may suffer for blobs with thick borders due to how approximation works there.
Test scene has been taken from CircularProgress and probably needs to be abstracted later.

@smoogipoo
Copy link
Contributor

I like that this is implemented as a separate shader, was scared it would get bunched into circular progress and that shader would become a hydra of sorts :)

if (pixelAngle < 0.0)
pixelAngle += TWO_PI;

int pointCount = 50;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be a const int instead. In older GLSL versions (< 4.00) there's no guarantee of these being "dynamically uniform" and from experience Intel drivers specifically do minimal optimisations.

// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

#nullable disable
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this + update any relevant code.

@peppy
Copy link
Sponsor Member

peppy commented Oct 27, 2022

Seems to be a bit broken with certain settings, not sure how easy this is to fix but:

osu Framework Tests 2022-10-27 at 03 20 55

osu.Framework.Tests.2022-10-27.at.03.21.13.mp4


namespace osu.Framework.Graphics.UserInterface
{
public class Blob : Sprite
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I'd probably call this CircularBlob or something else. Just Blob is too generic and has a different meaning in development terms.

@EVAST9919
Copy link
Contributor Author

EVAST9919 commented Oct 27, 2022

Seems to be a bit broken with certain settings, not sure how easy this is to fix

Yep, this is due to how we are approximating the path: we are plotting pointCount on a path within the search range and if it becomes too steep - this is the result.

Potential solution: increasing pointCount with the frequency, but iterating over potentially 1k or more points hurts the performance a lot.
Another solution would be pr'ing this into the osu-catch project directly (if it good enough for your needs) and not exposing this component for "wide use"

@peppy
Copy link
Sponsor Member

peppy commented Oct 27, 2022

I think it's a pretty good example component to have in the framework, but would be okay with either direction. Interested in @smoogipoo's thoughts.

@smoogipoo
Copy link
Contributor

I'm fine with it being in o!f.

@peppy
Copy link
Sponsor Member

peppy commented Oct 27, 2022

Ignoring the issue mentioned in discord regarding the seed (RNG.Next() set to the seed causes all perfect circles; using RNG.Next(0,200) to work around this for now) I've updated the osu!catch argon implementation to use this branch and it's looking pretty perfect:

osu.2022-10-27.at.04.14.02.mp4

@EVAST9919
Copy link
Contributor Author

Okay, I don't see the shape issue being fixed with the current implementation, needs a big alg rethinking.
The rest have been addressed so far.

@peppy peppy self-requested a review October 27, 2022 09:06
@peppy
Copy link
Sponsor Member

peppy commented Oct 27, 2022

[runtime] 2022-10-27 09:15:13 [error]: An unhandled error has occurred.
[runtime] 2022-10-27 09:15:13 [error]: osu.Framework.Graphics.OpenGL.Shaders.GLShader+PartCompilationFailedException: A osu.Framework.Graphics.OpenGL.Shaders.GLShaderPart failed to compile: sh_CircularBlob.fs:
[runtime] 2022-10-27 09:15:13 [error]: ERROR: 0:116: '*' does not operate on 'int' and 'float'
[runtime] 2022-10-27 09:15:13 [error]: ERROR: 0:116: '*' does not operate on 'int' and 'float'
[runtime] 2022-10-27 09:15:13 [error]: ERROR: 0:123: Use of undeclared identifier 'noisePosition'
[runtime] 2022-10-27 09:15:13 [error]: ERROR: 0:124: Use of undeclared identifier 'noiseValue'
[runtime] 2022-10-27 09:15:13 [error]: ERROR: 0:125: Use of undeclared identifier 'pos'
[runtime] 2022-10-27 09:15:13 [error]: at osu.Framework.Graphics.OpenGL.Shaders.GLShaderPart.Compile() in /Users/dean/Projects/osu-framework/osu.Framework/Graphics/OpenGL/Shaders/GLShaderPart.cs:line 151

@peppy peppy enabled auto-merge October 27, 2022 09:47
@peppy peppy merged commit be899c5 into ppy:master Oct 27, 2022
@EVAST9919 EVAST9919 deleted the blobs-pr branch October 27, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants