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

[shapes] Missing pixels on DrawCircle() with small radius and window FLAG_MSAA_4X_HINT active #3247

Closed
evanrinehart opened this issue Aug 17, 2023 · 19 comments

Comments

@evanrinehart
Copy link

#include <raylib.h>

int main(int argc, char* argv[]){

    SetConfigFlags(FLAG_MSAA_4X_HINT);
    InitWindow(800, 600, "TEST CASE");

    while(!WindowShouldClose()){
        BeginDrawing();
        ClearBackground(WHITE);
        DrawCircle(400.5, 300.5, 8, BLACK);
        DrawCircle(528.0, 172.0, 8, BLACK);
        EndDrawing();
    }

    CloseWindow();

    return 0;
}

image

This is on latest Ubuntu with NVIDIA GeForce GTX 1080 Ti. It seems like the constituents of the disc don't line up exactly and MSAA_4X has found those spots not covered.

@evanrinehart evanrinehart changed the title [Shapes] DrawCircle with smallish radius and FLAG_MSAA_4X_HINT results in results missing some pixels [Shapes] DrawCircle with smallish radius and FLAG_MSAA_4X_HINT results missing some pixels Aug 17, 2023
@evanrinehart
Copy link
Author

I narrowed it down to the rshapes.c routine DrawCircleSector. Specifically the SUPPORT_QUADS_DRAW_MODE branch of that code. When I force the triangles version of DrawCircleSector to draw my discs, the visual errors go away.

Something is going wrong when trying to draw circles with quads

@raysan5
Copy link
Owner

raysan5 commented Aug 18, 2023

@evanrinehart I'm aware of this issue. I don't know why it happens...

@raysan5 raysan5 changed the title [Shapes] DrawCircle with smallish radius and FLAG_MSAA_4X_HINT results missing some pixels [shapes] Missing pixels on DrawCircle() with small radius and window FLAG_MSAA_4X_HINT active Aug 19, 2023
@krupitskas
Copy link

krupitskas commented Aug 25, 2023

Can it be OpenGL bug?
However I've tried to compile raylib with 4.3 opengl, no difference.

I've briefly took a look:
When we are drawing to MSAA target samples look like this:
Sample 0:
image
Sample 1:
image
Other samples are having same issues, thats why resolve does look like this:
image

However I've checked shader and looks like issue with sampling of provided texture in fragment shader

    vec4 texelColor = texture(texture0, fragTexCoord);   
    finalColor = texelColor*colDiffuse*fragColor;        

I see that texture0 is R8G8_Unorm with RRRG swizzling, so I've tried to replicate same sampling in the shader

vec4 texelColor = texture(texture0, fragTexCoord).rrrg;   

And its gone...

image

With bigger circles it happens too
image

@raysan5
Copy link
Owner

raysan5 commented Aug 27, 2023

@krupitskas thanks for the investigation and the detailed explanation. I'm not sure about the reasons for this issue but my guess is that could be related to the rasterization of the individual indexed quads that become 2 triangles of the circle each. It just approximates the mathematical defined borders to pixels and on some cases it choses not to add a pixel.

The solution to this issue is drawing using plain triangles or a triangle-strip but it's not possible to get it with quads...

It's a guess but I don't really understand the issue, probably I don't really understand what MSAAx4 really does internally (beside the theorical definition).

@raysan5
Copy link
Owner

raysan5 commented Sep 2, 2023

I'm afraid this issue is out-of-scope of raylib. It seems it's more related to the driver implementation of OpenGL.

@raysan5 raysan5 closed this as completed Sep 2, 2023
@evanrinehart
Copy link
Author

I got the exact effect on my NVIDIA linux box and macbook air. I think a different way of drawing a circle with quads might help.

Currently I'm disabling SUPPORT_QUADS_DRAW_MODE in config.h which is enabled by default. It's a shame the default behavior is bugged.

I might be able to look at an alternative circle-with-quads drawing strategy at some point. I'll have to remember this closed issue.

@evanrinehart
Copy link
Author

Well after a few minutes of messing around I noticed that disabling the 1x1 pixel texture that goes with each slice of DrawCircleSector, the erroneous results seem to go away. I'll have to do more testing.

In the mean time, what is the purpose of the dummy texture used in all the QUAD mode rshapes routines?

@raysan5
Copy link
Owner

raysan5 commented Sep 4, 2023

I might be able to look at an alternative circle-with-quads drawing strategy at some point.

I tried this for a couple of hours and I couldn't find a solution.

what is the purpose of the dummy texture used in all the QUAD mode rshapes routines?

because raylib uses the same internal shader for shapes drawing and textures drawing, and that shader requires a texture. Also note that by default the font texture is used for shapes drawing, not the 1x1 pixel white texture.

@raysan5 raysan5 reopened this Sep 4, 2023
@raysan5
Copy link
Owner

raysan5 commented Sep 4, 2023

I'm reopening it to ask the X experts about it, maybe someone could provide some more info about the issue.

@mausimus
Copy link
Contributor

mausimus commented Sep 4, 2023

I think it's texel bleeding, the only material difference between triangle and quad approaches are tex coords (constant w/ triangles, varied w/ quads). I think there's already some padding being added around the shape area on the font texture but it looks not enough for all MSAA scenarios (an MSAA sample of a small shape could resolve to more than 1 texel away). Setting texShapesRec to width 1 and height 1 (or 0 for that matter) fixes it for example.

@raysan5
Copy link
Owner

raysan5 commented Sep 4, 2023

@mausimus you are right! thank you very much for finding it! Indeed there is a 1px white padding around the texture piece used for drawing but it seems it's not enough for MSAAx4! Reviewing it right now!

@raysan5 raysan5 closed this as completed Sep 4, 2023
@raysan5 raysan5 reopened this Sep 4, 2023
@raysan5
Copy link
Owner

raysan5 commented Sep 4, 2023

Not completed yet, just noticed that as smaller is the circle, the bigger should be the padding... very weird issue...

EDIT: It works, it was a bug on my fixing code. Just noted that padding should be bigger as smaller become the quads.

@raysan5 raysan5 closed this as completed in bdda1ef Sep 4, 2023
@evanrinehart
Copy link
Author

evanrinehart commented Sep 4, 2023

OH! The font texture. Of course. I'm glad you guys figured it out. My circles look great after pulling master and rebuilding.

@mausimus
Copy link
Contributor

mausimus commented Sep 4, 2023

You're welcome @raysan5 @evanrinehart ! Yes the problematic scenario is when a triangle doesn't cover the center of a pixel but covers one or more MSAA samples within it - the pixel shader is run only once using center of the pixel (which being outside of shape extrapolates UV to out of bounds) and its color output is used by all MSAA samples.

raysan5 added a commit that referenced this issue Sep 17, 2023
* Prettified a comment

* fixed broken indentation caused by another commit.
the commit renamed a bool to int and broke indentation: 233cf39

* Changed 0.001 and 0.00001 to EPSILON
This commit is untested.
I don't know what consequences this has.
Since the commits that added these numbers were before epsilon was added,
I have assumed that epsilon could replace them.

* Prettied up indentation in a few places

* removed spacing around *, standardizing it.

* I may have gotten overboard with indentation

* removed a few useless parenthesis

* Added fortran-raylib

* Fix examples/others/rlgl_standalone.c compilation issue (#3242)

* Update BINDINGS.md

* Ignore unused return value of GetCodepointNext in GetCodepointCount (#3241)

* Ignore unused return value of GetCodepointNext in GetCodepointCount

Removes the last warning from non-external libraries when compiling with
the default build configuration on x64 Linux.

* Remove unnecessary void cast in GetCodepointCount

* Fix #3246

* Revert "Fix #3246"

This reverts commit e4dcbd5.

* Fix text_unicode.c example crashing (#3250)

* Fix text_unicode.c example crashing

* Adjust the text_unicode.c example crashing fix

* tweaks

* add build.zig options for individual modules (#3254)

* Add `IsKeyPressedRepeat` (desktop only) (#3245)

Since the key pressed are handle by comparing current vs previous
state (ie frame), a special way is needed to handle key repeats.

* Reviewed `IsKeyPressedRepeat()` #3248

* Update rcore.c (#3255)

* Match CMakeOptions.txt options default values (#3258)

* Fix SetClipboardText for web (#3257)

* [Image] Validate that ImageDrawRectangleRec is drawing entirely inside the image (#3264)

* Add a function to clone a sound and share data with another sound.

* rename items based on feedback

* PR Feedback, use custom unload for sound alias, not variant of normal sound unloading

* sound_multi example

* Validate that image rect drawing is inside the image so we don't overflow a buffer

* remove files that should not have been added.

* remove changes that should not have been

* revert

* adsfasdfsdfsdf

* Add Vector3 Projecting and Rejection to Raymath (#3263)

* Update raymath.h

* formatting

* [Feature] IsKey... safety checks and more (#3256)

* [Feature] Add GetKeyRepeat

* Update rcore.c

* Simpler design, only one repeat per frame

* Update config.h

* Update rcore.c

* Add KEYBOARD_KEYS_MASK

* Update config.h

* reversions

* Update rcore.c

* Update rcore.c

* change docs

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update raylib.h

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Fix bug where default shaders was not linking. (#3261)

* Formating review

* Add missing cmake options (#3267)

* Fix CMake extraneous -lglfw (#3266)

Closes #3265.

The problem: LIBS_PRIVATE is a list of library names (used by pkg-config), but the shared library of the same name doesn't always exist.

* Fix example/models/models_loading_gltf.c controls (#3268)

* Fix example/models/models_loading_m3d.c controls (#3269)

* Remove e from secondes (#3270)

* Fix example/audio/audio_module_player.c help instructions and small bug (#3272)

* Fix example/audio/audio_module_player.c help instructions and small bug

* Update example/audio/audio_module_player.png screenshot

* Use type name instead of valid specifier

long long --> long long int

* REVIEWED: `GetFileLength()`, added comment #3262

* Update examples/models/models_loading_gltf.png;m3d.png screenshots (#3273)

* Remove a duplicated screenshot and add missing one (#3275)

* Add examples/shaders/shaders_lightmap.c to Makefiles (#3276)

* Fix examples/others/easings_testbed.c help instructions and small tweak (#3277)

* Fix examples/shaders/shaders_texture_outline.c help instructions (#3278)

* Fix examples/shapes/shapes_collision_area.c help instructions (#3279)

* RENAMED: LoadFont*() parameter names for consistency and coherence

* Fix uninitialized thread-locals in stbi #3282 (#3283)

* REVIEWED: Added `SetTextLineSpacing()` to multiline examples

* REVIEWED: Data size type consistency between functions #3168

* Some tweaks

* Use internal default allocators, instead of user-exposed ones

* Added rudimentary SVG support. (#2738)

* Added rudimentary SVG support. Added 2 functions ImageLoadSvg and ImageLoadSvgWithSize.

* Added an example on how to use ImageLoadSvgWithSize and adjusted Makefiles accordingly.

* Added actual correct example file.

* Reviewed the code to keep the raylib coding conventions in mind.
Moved the LoadImageSvg() code into LoadImage() guarded by SUPPORT_FILEFORMAT_SVG.
Renamed LoadImageSvgWithSize() to LoadImageSvg().
Added a LoadImageSvgFromString() function to parse the loaded SVG into an actual image. This does the bulk of the work.

* Fixed typo.

---------

Co-authored-by: Ray <[email protected]>

* REVIEWED: `LoadImageSvg()`

* REVIEWED: `LoadImageSvg()`

* Add SUPPORT_FILEFORMAT_SVG to cmake (#3284)

* Fix examples/textures/textures_fog_of_war.c help instructions (#3285)

* Fix examples/textures/textures_image_rotate.c help instructions (#3286)

* Update rtextures.c

* Fix #3247

* Update config.h

* Fix #3293

* Disable UBSAN in zig builds. (#3292)

Zig debug builds automatically enable ubsan.
As the fix for #1891 had to be reverted, debug builds using zig will crash like so:

```
Illegal instruction at address 0x3237d2
raylib/src/rlgl.h:3690:91: 0x3237d2 in rlDrawVertexArrayElements (/home/rcorre/src/raylib-zig-template/raylib/src/rcore.c)
    glDrawElements(GL_TRIANGLES, count, GL_UNSIGNED_SHORT, (const unsigned short *)buffer + offset);
```

This disables UBSAN when using zig to build raylib.

* Update README.md (#3290)

specially -> especially

* Update cmake SUPPORT_FILEFORMAT_SVG default value (#3291)

* Mouse offset and scaling must be considered also on web!

* Update rcore.c

* Update Makefile : clean raygui.c & physac.c (#3296)

* Remove PLATFORM_RPI (#3232)

* Remove PLATFORM_RPI

* remove build artifacts

---------

Co-authored-by: MichaelFiber <[email protected]>
Co-authored-by: Ray <[email protected]>

* Review to avoid UBSAN complaining #1891

* added raylib-raku to bindings (#3299)

* examples: core: adds 2D camera two player split screen (#3298)

* Reviewed examples for consistency

* Update rtext.c

* Some code restructuring for input functions, consistency review

* Remove unneeded #if (#3301)

Co-authored-by: MichaelFiber <[email protected]>

* Revert "Disable UBSAN in zig builds. (#3292)" (#3303)

This reverts commit a316f9e.

Issue #1891 was fixed again, so this is no longer needed.

* rtextures: Fix ImageDraw() source clipping when drawing beyond top left (#3306)

* REVIEWED: `TextToPascal()` issue when first char is uppercase

* Implement FLAG_WINDOW_RESIZABLE for web (#3305)

Fixes #3231

* Update BINDINGS.md (#3307)

Fix Kaylib binding. Reroute to a new repository.
Binding renamed.

* Update webassembly.yml

* Add claw-raylib to BINDINGS.md (#3310)

* Add SetWindowMaxSize for desktop and web (#3309)

* Add SetWindowMaxSize for desktop and web

* Remove SizeInt and respective adjustments

* Update rtextures.c

* Reviewed parameters for consistency

* Rename windowM* to screenM* (#3312)

* Update BINDINGS.md (#3317)

Update TurboRaylib bindings

* Update rmodels.c

* Update BINDINGS.md with vaiorabbit/raylib-bindings (#3318)

* fixed spelling mistake

* put back parenthesis

* reverted major allignment changes

* reverted parser output changes

* reverted one more indentation change

---------

Co-authored-by: Brian-E <[email protected]>
Co-authored-by: Ray <[email protected]>
Co-authored-by: ubkp <[email protected]>
Co-authored-by: ashn <[email protected]>
Co-authored-by: actondev (Christos) <[email protected]>
Co-authored-by: vitopigno <[email protected]>
Co-authored-by: Asdqwe <[email protected]>
Co-authored-by: Jeffery Myers <[email protected]>
Co-authored-by: Ethan Simpson <[email protected]>
Co-authored-by: Nickolas McDonald <[email protected]>
Co-authored-by: Branimir Ričko <[email protected]>
Co-authored-by: iacore <[email protected]>
Co-authored-by: Ethan Conneely <[email protected]>
Co-authored-by: Johannes Barthelmes <[email protected]>
Co-authored-by: bXi <[email protected]>
Co-authored-by: Ryan Roden-Corrent <[email protected]>
Co-authored-by: Ikko Eltociear Ashimine <[email protected]>
Co-authored-by: SuperUserNameMan <[email protected]>
Co-authored-by: MichaelFiber <[email protected]>
Co-authored-by: MichaelFiber <[email protected]>
Co-authored-by: Dan Vu <[email protected]>
Co-authored-by: Gabriel dos Santos Sanches <[email protected]>
Co-authored-by: Rob Loach <[email protected]>
Co-authored-by: Peter0x44 <[email protected]>
Co-authored-by: Kenta <[email protected]>
Co-authored-by: bohonghuang <[email protected]>
Co-authored-by: turborium <[email protected]>
Co-authored-by: Wilson Silva <[email protected]>
@verioneuvien
Copy link

verioneuvien commented Apr 21, 2024

This bug reappears in the DrawRectangleRounded function:

R: Radius (converted to roundness for DrawRectangleRounded's parameter, which in turn is converted back to radius)
S: Segments (for DrawRectangleRounded's parameter)
image

Image generated with the below code

#include <raylib.h>

int main()
{
	SetConfigFlags(FLAG_MSAA_4X_HINT);
	InitWindow(800, 400, "Rounded Rectangle Pixel Awkward\n");

	int segmentss[4] = { 1, 30, 45, 90 };
	int radiuss[4] = { 5, 10, 20, 40 };

	while (!WindowShouldClose())
	{
		BeginDrawing();
		ClearBackground(WHITE);

		for (int s = 0; s < 4; s++)
		{
			for (int r = 0; r < 4; r++)
			{
				// Calculate position and stuff
				Rectangle rec = { s * 200.0f + 20.0f, r * 100.0f + 10.0f, 160.0f, 80.0f };
				float roundness = radiuss[r] * 2.0f / rec.height; // WHY??

				// Main thing
				DrawRectangleRounded(rec, roundness, segmentss[s], BLACK);

				// Draw text on it
				const char *text = TextFormat("R:%d,S:%d", radiuss[r], segmentss[s]);
				float textWidth = MeasureText(text, 20);
				DrawText(text, rec.x + (rec.width - textWidth) / 2, rec.y + (rec.height - 20) / 2, 20, WHITE);
			}
		}

		EndDrawing();
	}

	CloseWindow();
}

System: Arch Linux x86_64
Raylib version: just pulled from master

Raylib logs:

INFO: Initializing raylib 5.1-dev
INFO: Platform backend: DESKTOP (GLFW)
INFO: Supported raylib modules:
INFO: > rcore:..... loaded (mandatory)
INFO: > rlgl:...... loaded (mandatory)
INFO: > rshapes:... loaded (optional)
INFO: > rtextures:. loaded (optional)
INFO: > rtext:..... loaded (optional)
INFO: > rmodels:... loaded (optional)
INFO: > raudio:.... loaded (optional)
INFO: DISPLAY: Trying to enable MSAA x4
INFO: DISPLAY: Device initialized successfully
INFO: > Display size: 1920 x 1080
INFO: > Screen size: 800 x 400
INFO: > Render size: 800 x 400
INFO: > Viewport offsets: 0, 0
WARNING: GLFW: Error: 65548 Description: Wayland: The platform does not support setting the window position
INFO: GLAD: OpenGL extensions loaded successfully
INFO: GL: Supported extensions count: 230
INFO: GL: OpenGL device information:
INFO: > Vendor: Intel
INFO: > Renderer: Mesa Intel(R) Xe Graphics (TGL GT2)
INFO: > Version: 4.6 (Core Profile) Mesa 24.0.5-arch1.1
INFO: > GLSL: 4.60
INFO: GL: VAO extension detected, VAO functions loaded successfully
INFO: GL: NPOT textures extension detected, full NPOT textures supported
INFO: GL: DXT compressed textures supported
INFO: GL: ETC2/EAC compressed textures supported
INFO: GLFW platform: Wayland
INFO: PLATFORM: DESKTOP (GLFW): Initialized successfully
INFO: TEXTURE: [ID 1] Texture loaded successfully (1x1 | R8G8B8A8 | 1 mipmaps)
INFO: TEXTURE: [ID 1] Default texture loaded successfully
INFO: SHADER: [ID 1] Vertex shader compiled successfully
INFO: SHADER: [ID 2] Fragment shader compiled successfully
INFO: SHADER: [ID 3] Program shader loaded successfully
INFO: SHADER: [ID 3] Default shader loaded successfully
INFO: RLGL: Render batch vertex buffers loaded successfully in RAM (CPU)
INFO: RLGL: Render batch vertex buffers loaded successfully in VRAM (GPU)
INFO: RLGL: Default OpenGL state initialized successfully
INFO: TEXTURE: [ID 2] Texture loaded successfully (128x128 | GRAY_ALPHA | 1 mipmaps)
INFO: FONT: Default font loaded successfully (224 glyphs)

@Cotterzz
Copy link

Cotterzz commented Apr 23, 2024

I've been looking at this and I am wondering if a different method of drawing curves might give improved rendering?
It's an odd way around the problem, but moving the lines to the edge instead of the centre might look better:
For illustration:
screenrec007
This is with MSAA turned on, though I'm on windows, no idea if this is the way to repeat the bug. Made it small and black for comparison. I'm using a custom mesh object:
screenrec001
Example code here:
https://github.com/Cotterzz/Raylib/blob/master/examples/alt_corner/rounded_corner_alternative.c

Also, let me know if this is more use drawn as a series of quads.

@Cotterzz
Copy link

Cotterzz commented May 7, 2024

This was an unexpected find, but it looks like rendering corners this way might be more performant:
https://www.humus.name/index.php?page=News&ID=228
He's doing something very similar with whole circles.
It's 15 years old, so I don't know if it still applies, but this was the only reference I could find to a similar method.

@raysan5
Copy link
Owner

raysan5 commented May 7, 2024

@Cotterzz This is an interesting approach, I didn't know about it! It could be an alternative but the code complexity required seems quite high compared to current solution.

@Cotterzz
Copy link

Cotterzz commented May 7, 2024

Yes, and it's a very specific use case for 2D only, so I can't really say if it's worth implementing.
My code is probably very overcomplicated as it was my first attempt at the algorithm, I'm not great with C, and I should probably have used recursion - so if you did want to use it, I'm sure it can be reduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants