Zombieschannel / SFML

Simple and Fast Multimedia Library
https://www.sfml-dev.org/
zlib License
1 stars 1 forks source link

Various small fixes #12

Closed Scorbutics closed 1 month ago

Scorbutics commented 2 months ago

Hello,

Following your comment posted in the issue https://github.com/Zombieschannel/SFML/issues/11, I've built a simpler list of commit that describe individually each task.

Those include:

Refactoring, not fixes:

Missing Feature:

Fixes:

Chores:

I've voluntary removed the c++17 code as you mentioned in the issue, to keep it compatible with c++03. The removed code was taking out the the "sf_texture" matrix computation from RenderTarget to put it in the Texture::bind method, where it's also done for classic OpenGL. If you are interested in doing that again (but using only c++03), do not hesitate to tell me.

I can also split this PR in several small ones, if you prefer, but it was easier for me to bundle all fixes I needed in one pull request in order to check my modifications were fine.

Unfortunately, I can't provide any minimum reproducible example code right now, as those issues were noticed while running a game with complex layers, it would required to invest some time to do it. But again, if you need it, I can do it. I just don't know when! Also, note that building the SFML shaders official example helped me found some of those fixes, so it might be enough.

Zombieschannel commented 2 months ago

Looking at this now, https://github.com/Zombieschannel/SFML/commit/bf97b36895e7cddf1b315e9bd05dc5bddd70e089 this change looks good except that I'm not quite sure GL_GEOMETRY_SHADER is defined on GLES2? Since afaik geometry shaders aren't supported in GLES in any version, so that is just kind of strange.

Wanted to ask what is the purpose of this gitignore change exactly? https://github.com/Zombieschannel/SFML/commit/1c765e0d00d99c27f0cf6d9048a3927b728afd9a

If you are interested in doing that again (but using only c++03), do not hesitate to tell me.

I wasn't really planning to change that, I'd like to keep the function parameters and return types the same as SFML 2.6.x since the main goal is for this branch to be a drop and replace for existing SFML code.

When it comes to this fix: https://github.com/Zombieschannel/SFML/commit/552e98ab0146388a0691fb4d134d2837b40eb715, I'd like to know what does it fix? Though since it is caching, I'm not sure how much it can be tested and likely the impact is small.

https://github.com/Zombieschannel/SFML/commit/c3786f4c3383b4ac2862ed26d9f9e58bc5872859 I've never really found nice default uniform names in shaders, but imo sf_sampler seemed quite solid though I'd like to hear from other people like @Nathan-M-code what they think.

I have nothing to add to these fixes: https://github.com/Zombieschannel/SFML/commit/8198317d09cf2ddc7bcf07703e06a4bb3f6df8a4 & https://github.com/Zombieschannel/SFML/commit/7efc4fdadc15c902fe994086dbd463afbb6f1717, all good.

Just wondering, does this fix https://github.com/Zombieschannel/SFML/commit/21f4e444a8f7e0c0db1a8534d2a051fb9f185f15 actually fix this issue (or at least it adds the needed uniforms, I know shader code hasn't changed)? https://github.com/Zombieschannel/SFML/issues/13

Nathan-M-code commented 2 months ago

I find

"uniform sampler2D texture;"
"uniform mat4 sf_texture;"

a bit confusing. sf_sampler was better imo

Scorbutics commented 2 months ago

Looking at this now, https://github.com/Zombieschannel/SFML/commit/bf97b36895e7cddf1b315e9bd05dc5bddd70e089 this change looks good except that I'm not quite sure GL_GEOMETRY_SHADER is defined on GLES2? Since afaik geometry shaders aren't supported in GLES in any version, so that is just kind of strange.

You're right, it might come from an aggressive copy paste on my side. As it's not compiled when SFML_OPENGL_ES is not defined, I didn't see it.

Wanted to ask what is the purpose of this gitignore change exactly? https://github.com/Zombieschannel/SFML/commit/1c765e0d00d99c27f0cf6d9048a3927b728afd9a

Each time I trigger a cmake configuration, it generates a lot of noise if I don't ignore them. Not sure why it was not ignored from the start, in the official SFML repo, but it's not related at all with OpenGL ES, so we can discard it if you want.

When it comes to this fix: https://github.com/Zombieschannel/SFML/commit/552e98ab0146388a0691fb4d134d2837b40eb715, I'd like to know what does it fix? Though since it is caching, I'm not sure how much it can be tested and likely the impact is small.

That's the tricky part... as I'm working on a full game engine port to Android, I was only able to do some tests on it. I noticed that in my case, vertices weren't update well and disabling the cache was necessarily. I will try to write a minimal reproducible example of code asap :)

https://github.com/Zombieschannel/SFML/commit/c3786f4c3383b4ac2862ed26d9f9e58bc5872859 I've never really found nice default uniform names in shaders, but imo sf_sampler seemed quite solid though I'd like to hear from other people like @Nathan-M-code what they think.

Oops, I thought this was badly named here but apparently texture is how we name sf_sampler in the engine I'm working on :sweat_smile:. I will remove that!

Just wondering, does this fix https://github.com/Zombieschannel/SFML/commit/21f4e444a8f7e0c0db1a8534d2a051fb9f185f15 actually fix this issue (or at least it adds the needed uniforms, I know shader code hasn't changed)? https://github.com/Zombieschannel/SFML/issues/13

It's certainly related as it allows to get "virtual" texture coordinates based on size, but I'm not sure it can fix this. I might have to check that. As an example of use:

#ifdef GL_ES
precision mediump float;

uniform mat4 sf_texture;
uniform vec2 factor_npot;

varying vec2 v_texCoord;
#else
const vec2 factor_npot = vec2(1.0, 1.0);
#endif

uniform float t;

const float PI = 3.14159265359;

void main() {
  vec4 COLORS[7];
  COLORS[0] = vec4(1.0000, 0.1490, 0.2196, 1.0);
  COLORS[1] = vec4(1.0000, 0.4196, 0.1882, 1.0);
  COLORS[2] = vec4(1.0000, 0.6353, 0.0078, 1.0);
  COLORS[3] = vec4(0.0078, 0.8157, 0.5686, 1.0);
  COLORS[4] = vec4(0.0039, 0.5020, 0.9843, 1.0);
  COLORS[5] = vec4(0.4824, 0.2118, 0.8549, 1.0);
  COLORS[6] = vec4(1.0000, 0.1490, 0.2196, 1.0);

#ifdef GL_ES
  vec2 tc = (sf_texture * vec4(v_texCoord / factor_npot, 0.0, 1.0)).xy;
#else
  vec2 tc = gl_TexCoord[0].xy;
#endif

  float circleX = mod((tc.x - t) * 6.0, 1.0);
  float colorIndex = mod((tc.x - t) * 6.0, 6.0);
  vec4 frag = mix(
    COLORS[int(floor(colorIndex))],
    COLORS[int(floor(colorIndex)) + 1],
    fract(colorIndex)
  );
  float circleRadius = pow(sin(tc.x * PI), 3.0);
  frag.a = 1.0 - (sqrt(pow((tc.y * 2.0 - 1.0) / circleRadius, 2.0) + pow(circleX * 2.0 - 1.0, 2.0)));
  gl_FragColor = frag;
}

will create this effect: image but without dividing by this factor, it becomes: image (the shader looks "cropped")

I won't be available for next two weeks so I try to build the example I mentioned quickly, otherwise I will be back for it the 16th of July.

Scorbutics commented 2 months ago

Ok, for the cache commit, I got something easily reproducible. From the root SFML directory, do the following:

pixelate.frag:

precision mediump float;

uniform sampler2D texture;
uniform float pixel_threshold;
uniform mat4 sf_texture;

varying vec2 v_texCoord;
varying vec4 v_color;

void main()
{
    vec4 coord = sf_texture * vec4(v_texCoord, 0.0, 1.0);
    float factor = 1.0 / (pixel_threshold + 0.001);
    vec2 pos = floor(coord.xy * factor + 0.5) / factor;
    gl_FragColor = texture2D(texture, pos) * v_color;
}

pixelate.vert:

attribute vec2 position;
attribute vec2 texCoord;
attribute vec4 color;

uniform mat4 sf_modelview;
uniform mat4 sf_projection;

varying vec2 v_texCoord;
varying vec4 v_color;

void main() {
    v_texCoord = texCoord;
    v_color = color;
    gl_Position = sf_projection * sf_modelview * vec4(position.xy, 0.0, 1.0);
}
 - Build and execute the `shader` example: `cmake --build . && (cd examples/shader && ./shader)`

You get the following result:
![image](https://github.com/Zombieschannel/SFML/assets/5864483/38a6c87d-3c7d-48d3-a444-e20130fa3dcf)

You can see the text box container not visible.

Then, edit `RenderTarget.cpp` and replace the following (l.795):
```diff

    if (useVertexCache)
    {
        // Since vertices are transformed, we must use an identity transform to render them
#ifndef SFML_OPENGL_ES
-        if (!m_cache.enable || !m_cache.useVertexCache)
+
        {
            glCheck(glLoadIdentity());

Building and executing the shader example, you can now see:

image

Better, but there is still a drawing problem...

Finally, edit RenderTarget.cpp by removing the following (l.354):

        // If we switch between non-cache and cache mode or enable texture
        // coordinates we need to set up the pointers to the vertices' components
-        if (!m_cache.enable || !useVertexCache || !m_cache.useVertexCache)
+
        {
            const char* data = reinterpret_cast<const char*>(vertices);

You'll be able to see the example fully working: image

Zombieschannel commented 1 month ago

All seems good! Can confirm it fixed the example with shaders. Also the factor_npot uniform (maybe not the best name but ok for now) will probably help me with this issue: https://github.com/Zombieschannel/SFML/issues/13

But yeah again, thanks a lot for these fixes!

Scorbutics commented 1 month ago

With pleasure. If by any means I find something else (I did not run all shader examples), I will tell you