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

Ability to render EdgeEffects only outside the draw rectangle #737

Merged
merged 14 commits into from May 21, 2017

Conversation

4 participants
@phosphene47
Contributor

phosphene47 commented May 21, 2017

#726
Looks a little ugly though

Show outdated Hide outdated osu.Framework/Resources/Shaders/sh_TextureRounded.fs
// Discard inner pixels
if (g_DiscardInner && dist <= g_CornerRadius - g_MaskingBlendRange)
{

This comment has been minimized.

@peppy

peppy May 21, 2017

Member

mixed spaces/tabs

@peppy

peppy May 21, 2017

Member

mixed spaces/tabs

@thomastanck

This comment has been minimized.

Show comment
Hide comment
@thomastanck

thomastanck May 21, 2017

Contributor

Can you add a test case for rounded containers?

Contributor

thomastanck commented May 21, 2017

Can you add a test case for rounded containers?

@phosphene47

This comment has been minimized.

Show comment
Hide comment
@phosphene47

phosphene47 May 21, 2017

Contributor

@thomastanck There is one already

Contributor

phosphene47 commented May 21, 2017

@thomastanck There is one already

@thomastanck

This comment has been minimized.

Show comment
Hide comment
@thomastanck

thomastanck May 21, 2017

Contributor

Container is rounded

            Add(new Container()
            {
                Size = new Vector2(200f, 100f),
                Position = new Vector2(100f),

                Masking = true,
                EdgeEffect = new EdgeEffect()
                {
                    Type = EdgeEffectType.Glow,
                    Colour = Color4.Khaki,
                    Radius = 100f,
                    Roundness = 0f,
                    Hollow = true
                },

                CornerRadius = 50f,

                Children = new Drawable[]
                {
                    new Box()
                    {
                        Size = new Vector2(200f, 100f),
                        Colour = Color4.Aqua,
                    },
                }
            });

EdgeEffect is rounded

            Add(new Container()
            {
                Size = new Vector2(200f, 100f),
                Position = new Vector2(100f),

                Masking = true,
                EdgeEffect = new EdgeEffect()
                {
                    Type = EdgeEffectType.Glow,
                    Colour = Color4.Khaki,
                    Radius = 100f,
                    Roundness = 60f,
                    Hollow = true
                },

                //CornerRadius = 50f,

                Children = new Drawable[]
                {
                    new Box()
                    {
                        Size = new Vector2(200f, 100f),
                        Colour = Color4.Aqua,
                    },
                }
            });
Contributor

thomastanck commented May 21, 2017

Container is rounded

            Add(new Container()
            {
                Size = new Vector2(200f, 100f),
                Position = new Vector2(100f),

                Masking = true,
                EdgeEffect = new EdgeEffect()
                {
                    Type = EdgeEffectType.Glow,
                    Colour = Color4.Khaki,
                    Radius = 100f,
                    Roundness = 0f,
                    Hollow = true
                },

                CornerRadius = 50f,

                Children = new Drawable[]
                {
                    new Box()
                    {
                        Size = new Vector2(200f, 100f),
                        Colour = Color4.Aqua,
                    },
                }
            });

EdgeEffect is rounded

            Add(new Container()
            {
                Size = new Vector2(200f, 100f),
                Position = new Vector2(100f),

                Masking = true,
                EdgeEffect = new EdgeEffect()
                {
                    Type = EdgeEffectType.Glow,
                    Colour = Color4.Khaki,
                    Radius = 100f,
                    Roundness = 60f,
                    Hollow = true
                },

                //CornerRadius = 50f,

                Children = new Drawable[]
                {
                    new Box()
                    {
                        Size = new Vector2(200f, 100f),
                        Colour = Color4.Aqua,
                    },
                }
            });
@phosphene47

This comment has been minimized.

Show comment
Hide comment
@phosphene47

phosphene47 May 21, 2017

Contributor

But that really can't be helped.
EdgeEffect is supposed to be rendered underneath everything.
That's a masking problem.

Contributor

phosphene47 commented May 21, 2017

But that really can't be helped.
EdgeEffect is supposed to be rendered underneath everything.
That's a masking problem.

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy May 21, 2017

Member

The top example showing a one pixel hole likely needs to be fixed at a shader level, though.

Member

peppy commented May 21, 2017

The top example showing a one pixel hole likely needs to be fixed at a shader level, though.

@phosphene47

This comment has been minimized.

Show comment
Hide comment
@phosphene47
Contributor

phosphene47 commented May 21, 2017

screenshot from 2017-05-21 13-22-39

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy May 21, 2017

Member

@paparony03 are you on discord per chance?

that's looking a lot better, but i recommend you reduce the corner radius in the visualtest as it's currently set high enough to become non-rounded at the converging points of each corner.

also i'd look at adding a coernerradius = 0 test, and then duplicating each of the two cornerradius displays with a transparent fill colour.

you could use a visualtest layout similar to TestCaseFillModes

Member

peppy commented May 21, 2017

@paparony03 are you on discord per chance?

that's looking a lot better, but i recommend you reduce the corner radius in the visualtest as it's currently set high enough to become non-rounded at the converging points of each corner.

also i'd look at adding a coernerradius = 0 test, and then duplicating each of the two cornerradius displays with a transparent fill colour.

you could use a visualtest layout similar to TestCaseFillModes

Show outdated Hide outdated osu.Framework/Resources/Shaders/sh_TextureRounded.fs
float dist = distanceFromRoundedRect();
// Discard inner pixels
if (g_DiscardInner && dist <= g_CornerRadius - g_MaskingBlendRange - 1) // -1 for getting rid of some ugly inner-edges

This comment has been minimized.

@peppy

peppy May 21, 2017

Member

have to explicitly use - 1.0 here or it will break on some drivers:

untitled

@peppy

peppy May 21, 2017

Member

have to explicitly use - 1.0 here or it will break on some drivers:

untitled

Show outdated Hide outdated osu.Framework.VisualTests/osu.Framework.VisualTests.csproj
@@ -1,109 +1,110 @@
<?xml version="1.0" encoding="utf-8"?>

This comment has been minimized.

@peppy

peppy May 21, 2017

Member

i think you changed the line endings on this file

@peppy

peppy May 21, 2017

Member

i think you changed the line endings on this file

Show outdated Hide outdated osu.Framework/Resources/Shaders/sh_TextureRounded.fs
// Discard inner pixels
if (g_DiscardInner && dist <= g_CornerRadius - g_MaskingBlendRange - 1) // -1 for getting rid of some ugly inner-edges
{
gl_FragColor = vec4(0.0);

This comment has been minimized.

@peppy

peppy May 21, 2017

Member

this is still spaces instead of tabs

@peppy

peppy May 21, 2017

Member

this is still spaces instead of tabs

@phosphene47

This comment has been minimized.

Show comment
Hide comment
@phosphene47

phosphene47 May 21, 2017

Contributor

LF or CRLF?

Contributor

phosphene47 commented May 21, 2017

LF or CRLF?

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy May 21, 2017

Member

match what was previously there.

Member

peppy commented May 21, 2017

match what was previously there.

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy May 21, 2017

Member

visual tests need to be logical. try this:

corner 0 / 0.5f colour fill | corner 50 / 0.5f colour fill
---------------------------------------------------------
corner 0 / 0f colour fill   | corner 50 / 0f colour fill

so you can actually see that hollow is working as expected

Member

peppy commented May 21, 2017

visual tests need to be logical. try this:

corner 0 / 0.5f colour fill | corner 50 / 0.5f colour fill
---------------------------------------------------------
corner 0 / 0f colour fill   | corner 50 / 0f colour fill

so you can actually see that hollow is working as expected

phosphene47 added some commits May 21, 2017

Ability to render EdgeEffects only outside the draw rectangle
Bad spacing


Oops bad csproj


Fix more ugly stuff


Better test case


Even better TestCase


Use 1.0


Use tabs


Better better TestCase


Bad line ending


Satisfy CodeFileSanity
@@ -653,6 +655,8 @@ public struct MaskingInfo : IEquatable<MaskingInfo>
public float BlendRange;
public float AlphaExponent;
public bool Hollow;
public bool Equals(MaskingInfo other)
{
return

This comment has been minimized.

@Tom94

Tom94 May 21, 2017

Collaborator

You are missing a Hollow comparison in the Equals override.

@Tom94

Tom94 May 21, 2017

Collaborator

You are missing a Hollow comparison in the Equals override.

@Tom94

This comment has been minimized.

Show comment
Hide comment
@Tom94

Tom94 May 21, 2017

Collaborator

Otherwise well done! The shader / DrawNode stuff is perfectly executed. :)

Collaborator

Tom94 commented May 21, 2017

Otherwise well done! The shader / DrawNode stuff is perfectly executed. :)

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy May 21, 2017

Member

👍 nice job

Member

peppy commented May 21, 2017

👍 nice job

@peppy

peppy approved these changes May 21, 2017

@peppy peppy merged commit 60e0a34 into ppy:master May 21, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment