Add support for custom texture

[JonathSpirit]

(note that this is a in progress pr and need testing)

Add the possibility to change the light texture with a custom one.

Notable change (RadialLight) :

• add getter/setter for fade and plain texture :

void setLightFadeTexture(sf::Texture& texture);

sf::Texture* getLightFadeTexture();

void setLightPlainTexture(sf::Texture& texture);

sf::Texture* getLightPlainTexture();

std::shared_ptr<sf::Texture> getDefaultLightFadeTexture() const;

std::shared_ptr<sf::Texture> getDefaultLightPlainTexture() const;

• replace RenderTexture with a simple texture :
• replace unique_ptr with shared_ptr to be able of retrieving default texture without problem :
• (note that this change should fix Mutex Lock crash when initialising RadialLight when linking static library. #10 when exiting the program)

std::shared_ptr<sf::Texture> l_lightTextureFade;
std::shared_ptr<sf::Texture> l_lightTexturePlain;

• add a m_localRadius that change according to input texture
• BASE_RADIUS is now an unsigned int instead of float :

const unsigned int BASE_RADIUS = 400;

• calculation of the pixels in initializeTextures is now smoother and math friendly

• in line 285 of RadialLight.cpp, the maximum range distance is reduced too avoid too far vertex for the polygon :

//before
points.push_back(tr_i.transformPoint(castRay(begin, end,    r, m_range*m_range)));
//after
points.push_back(tr_i.transformPoint(castRay(begin, end, r, m_range*2)));

---

Known problem :

When the texture have non-transparent pixels at his border, a bright glitch can appear. This is due to the coloured (in this example white) vertex that are too far from the texture bound, this will cause OpenGL to draw some weird pixel in order to fill the entire polygon.

To avoid that, I was thinking about 3 solutions :

• The user must pass texture that have a 1 pixel transparent border
• Having a custom function that load an image, add the transparent border and convert it to texture
• Maybe fragment shader can help

---

Screenshots/GIFs :

no border :

with transparent border :

---

How to test (what I do) :

in the demo.cpp file

I added a global variable :

sf::Texture* test;

At line 369 :

reinterpret_cast<candle::RadialLight*>(nl.get())->setLightFadeTexture(*test);

In the main :

test = new sf::Texture();
test->loadFromFile("buisness_cat2.png");

---

I'm open to suggestion, fix, critic etc.... 😃

[MiguelMJ]

I think this is a potentially great new feature.

First, the small things.
Your way to create the default textures using Images instead of RenderTextures is far better than the current one. If you wanted to isolate that in a separate PR I would merge it immediatlely. Also, I would have to re-read the problems with the crash and the issue in SFML, but its probable that the change would fix it, for what I remember.

Now, about the real proposal.
Adding a new attribute to the light is a big change and, following the current philosophy of the project, there are several important things to take into account.

• Conceptually speaking, the light should only have one texture (even though behind there were two, as right now). So we would need to think of a design where there are not setLightFadeTexture and setLightPlainTexture but only setTexture.
  • This restriction could also be the answer to the problem of the 1px transparent border: the setter could assign the given texture to the m_lightTexturePlain and then make a processed copy to make m_lightTextureFade (applying the fade effect and the transparent border). However, we would need to be very careful managing this inner resource...
• There would have to be functions to customize the texture. The bar is set by the texture of LightingArea, so the minimum function I would require before release would be setTextureRect.
  • This reminds me that I have to add more texture related functions like setTextureRotation(float), setTextureRepeated(bool). Not urgent.
• The texture attribute should belong to the parent class, LightSource and not be specific to RadialLight. I wouldn't include it in a new release before texture was implemented in DirectedLight.

As those are quite a few changes I would even have a separate branch to work on it in the main repo.
Thanks!

[MiguelMJ]

Some notes:
About the first point. I'm not sure about the getters to the default textures (that, btw, should be static). The only use case that I can think for that functions is using the returned value in setTexture, but that would mean two exceptions to its regular behavior... What about overload setTexture without arguments for setting the default values?
About the new branch, actually if I had something to add I guess I'd do it in yours 😅

[JonathSpirit]

Some notes: About the first point. I'm not sure about the getters to the default textures (that, btw, should be static). The only use case that I can think for that functions is using the returned value in setTexture, but that would mean two exceptions to its regular behavior... What about overload setTexture without arguments for setting the default values?

Maybe we can put a setTexture(sf::Texture* texture) and if argument is nullptr, the default one would be taken.

About the new branch, actually if I had something to add I guess I'd do it in yours 😅

no problem 👍

First, the small things. Your way to create the default textures using Images instead of RenderTextures is far better than the current one. If you wanted to isolate that in a separate PR I would merge it immediatlely.

Mmh I wanted to do a all in one pr, but if you confirm to me that you want another pr, yeah I can.

* **Conceptually speaking, the light should only have one texture** (even though behind there were two, as right now). So we would need to think of a design where there are not `setLightFadeTexture` and `setLightPlainTexture` but only `setTexture`.

Maybe we can add a flag : bool m_customTexture
when you add a custom texture with setTexture, the fade flag will be useless and the custom texture will always be used, and when you reset to the default one, fade flag will be able to switch between the two.

  * This restriction could also be the **answer to the problem of the 1px transparent border**: the setter could assign the given texture to the `m_lightTexturePlain` and then make a processed copy to make `m_lightTextureFade` (applying the fade effect and the transparent border). However, we would need to be very careful managing this inner resource...

I don't like the idea of copying texture, imagine that for 100 lights the same texture is copied again for all of them. I don't think this is memory friendly.

* There would have to be functions to **customize the texture**. The bar is set by the texture of `LightingArea`, so the minimum function I would require before release would be `setTextureRect`.

Yep I agree with that, this can also lead to rectangle RadialLight for more user customisation

  * _This reminds me that I have to add more texture related functions like `setTextureRotation(float)`, `setTextureRepeated(bool)`. Not urgent._

I would like to add a setBlendMode too, this can lead to fully colored light texture, again for more user customisation

* The **texture attribute should belong to the parent class**, LightSource and not be specific to RadialLight. I wouldn't include it in a new release before texture was implemented in DirectedLight.

Yep agree with that

[MiguelMJ]

Maybe we can put a setTexture(sf::Texture* texture) and if argument is nullptr, the default one would be taken.

OK, I like this idea better.

Mmh I wanted to do a all in one pr, but if you confirm to me that you want another pr, yeah I can.

There is no hurry, we can do all this in the same pr. If that's easier for you, no problem :D

I don't like the idea of copying texture, imagine that for 100 lights the same texture is copied again for all of them. I don't think this is memory friendly.

I was thinking to have an inner static caché of textures static std::map<sf::Texture*, sf::Texture*> s_fadedTexturesCache that maps a texture to its processed fade version. The first time you use a texture, it is processed and copied, then the pointer of the original is stored in the cache. Then, if the texture is used in another light, before copying again the texture, it checks if a pointer to this texture is already in the cache, and if so, then use the existing processed texture for fade.
Somewhat like this:

void RadialLight::setTexture(sf::Texture* texture){
    if(texture == nullptr){
        // ...
    }
    m_lightTexturePlain = texture;
    auto cachedFadeTextureIter = s_fadedTexturesCache.find(texture);
    if(cachedFadeTextureIter != s_fadedTexturesCache.end()){
        m_lightTextureFade = *cachedFadeTextureIter;
    }else{
        m_lightTextureFade = makeFadeTexture(texture);
        s_fadedTexturesCache[texture] = m_lightTextureFade;
    }
}

Maybe we can add a flag : bool m_customTexture
when you add a custom texture with setTexture, the fade flag will be useless and the custom texture will always be used, and when you reset to the default one, fade flag will be able to switch between the two.

The problem I see with this is that, as a user, it makes sense that the texture and the fade flag can interact:

-	default texture	custom texture
fade=false	default plain texture	texture as given
fade=true	default fade texture	same texture but fading

At least, this is the behavior I would expect as a user.

I would like to add a setBlendMode too, this can lead to fully colored light texture, again for more user customisation

Yes, why not. Only that we would have to be careful on how this would interact with a LightingArea.

---

Side note: I'm sure that all this could be easily solved by shaders. The day we can get a contributors that knows GLSL this is going to have a huge revamp 😂

Edit: minor change in snippet

[JonathSpirit]

Mmmh I agree, shaders will be so good in this case x), I think I can try some GLSL this should not be too hard 😃

[MiguelMJ]

I just found an interesting repository with a similar project:
https://github.com/DaiMysha/LightSystem/tree/dev
Although it has a different approach, it has really neat ideas I'd like to study in the future for this Candle. In fact, it might have the shader we need here.
I don't know if @DaiMysha will be interested in Candle, but I think it's worth it at least giving them a big shoutout for the inspiration!

[DaiMysha]

Hello, thank you for the shoutout !

I started working on my own light system due to specific needs i had regarding functionalities. it seems that my version is slightly more complete than yours, but also has a completely different approach and is more geared towards scenes with high numbers of lights. I haven't had time to work on it due to other projects being in the pipelines so the repo might appear dead. I have a big number of things i want to add to it however, including optimization.

While i am not interested in actively taking part in the development of Candle due to having my own system at the moment, i'd be happy to discuss your objectives with your system (maybe see if my version has everything you need ?) and eventually provide help or technical assistance.

If not, and if you end up reusing some of my ideas or code, i'd like to at least be credited in your repository.

[MiguelMJ]

Thank you, @DaiMysha ! In case I take some non-trivial concept from your system, I'll credit you in the category ideas and if I take a piece of your code, also in code.

Having a look at your project, I think that the biggest difference is that Candle is not properly a lighting system, but needs the user to build their own. If you wanted a scene with lots of lights, with Candle you'd have to manage which ones get updated and which one do not, while your system manages that semi-automatically, I think. I will think about this for future versions...

Anyways, thanks again ❤️ . If there's anything I wanted to discuss about this, I'll open a specific issue, to avoid bloating this thread.

[JonathSpirit]

Candle you'd have to manage which ones get updated and which one do not

This is what I love with Candle, I can simply build a framework/engine around the light system :)

I will check the code from the mentioned repo and see if I can try something, thanks !

[JonathSpirit]

I found a simple solution for the border problem here :

SFML do a GL_CLAMP_TO_EDGE but what we want is GL_CLAMP_TO_BORDER and it seems that SFML doesn't provide an option for that.

This can be a really simple solution.

https://learnopengl.com/Getting-started/Textures

[MiguelMJ]

There must be a way to change that value... I guess from the shader it would be possible(?)

[DJuego]

Hi! A note of interest in this regard I hope:

Another project of great interest, that I believe to be the Light System forerunner in the SFML ecosystem, is LTBL2. You may already know it. Although old it also proved to be very powerful. It is a pity that the great @222364 has not shown signs of life for such a long time...

Best wishes for Candle!

DJuego

[MiguelMJ]

Thanks, DJuego!
I know LTBL and, interestingly, the fact that it didn't convinced me was one of the several reasons I even published Candle. LTBL is a much more mature project, but I didn't feel it was for me... I can't point exactly what it is, but I just wanted a different approach.
However, I feel big respect for the project and there are definitely some things,like soft shadows, that I plan to study in the future, inspired by it. If I only had the time!

[JonathSpirit]

I tested a bit with GLSL, but It's hard to find something that work.
My goal with this test is just simply show a texture with a fragment shader but I don't really know how to do that correctly.

#version 140

uniform sampler2D u_texture;
uniform vec2 u_resolution;
uniform vec2 u_position;
uniform vec2 u_size;

out vec4 diffuseColor;
in vec4 gl_Color;

void main(){
    vec2 coord = gl_FragCoord.xy;
    coord.y = u_resolution.y - coord.y;
    ivec2 textureSize2d = textureSize(u_texture,0);
    vec2 textureSize = vec2(textureSize2d);
    
    vec4 color = texture2D(u_texture, (coord-u_position) / u_size);
    diffuseColor = gl_Color * vec4(color);
}

If someone want to help (in fact I don't really have the time to work with GLSL).

[MiguelMJ]

I see... well, thanks for the attempt, @JonathSpirit . I'm gonna move this to another issue, keep the approach you want for this specific issue with everything we talked about in mind. I wish there were more clear tutorials for SFML+GLSL.

[JonathSpirit]

I don't really like that we have to manage textures resource, maybe we can have a special structure like this :

struct LightTexture
{
     sf::Texture _plain;
     sf::Texture _fade;
};

And have a function for light texture loading :

std::shared_ptr<LightTexture> LoadLightTextureFromFile(const std::string& path);

This let the user the responsibility to manage memory resource.

The last commit that I push is here to show an implementation with a std::map and std::pair (for the plain/fade texture)

[MiguelMJ]

Mmm I would not add a new structure to store light textures and let the user manage it. In part because if Candle uses shaders in the future, that could cause compatibility problems between Candle versions (as the LightTexture struct wouldn't be needed once shaders enter), so we have to keep the user managing a single texture resource. The idea is to keep the API as robust as possible.
Your implementation in the last commit looks like it should work fine! Have you tried it? I guess that the memory could be a problem if you used a lot of different light textures, but there should be no problem (in that sense) using lots of lights with a few textures...
Let me know if you test it.
Thank you!

[JonathSpirit]

Yep it work well I think now it's a little bit of cleaning. Should we free the generated textures at some point ?

I tested to put a texture on directed light but it's not that good :

[MiguelMJ]

Right now I'm not sure but maybe the problem in directed light is that the castLight function does not update the texCoords at all.
About freeing the textures... I think that in most cases they could just be there until the end of the execution, but we could add a cleanup(sf::Texture*) function to allow the user to free the generated textures manually. Even if a future version with shaders doesn't use it, it is better to just deprecate a function than a whole data type.

[JonathSpirit]

Note that for non-perfect square texture, the texture don't stretch causing the light to be not centered on the texture center.

I tried to set setRepeated(true); on the textures but this is not working :

I tried to update texture coord for directed light but doesn't seems to work :

[JonathSpirit]

Add a cleanupTexture(sf::texture* texture)
when nullptr, every cached texture is cleaned

[JonathSpirit]

I correctly put a fade effect and at the same time in the function setTexture() I manually do a repeated pattern for texture that is not square

(the used image isn't really visible)

[JonathSpirit]

Test with original texture :

[JonathSpirit]

Test on a smaller size y texture :

[JonathSpirit]

The current status of this pr :

I tried to implement textures into DirectedLight but I can't find a way for this to work (a bit lost on that). But overall, this work well on RadialLight.

The bar is set by the texture of LightingArea, so the minimum function I would require before release would be setTextureRect.

I do not see how setTextureRect can be implemented here, the texture is bound to every points of the polygon, I think that if we want to customise the texture rect, we have to use shaders.

Maybe I shouldn't have tried to implement a new feature but more to focus on optimization/cleaning because it becomes a bit complicated to navigate through the code.

First, the small things. Your way to create the default textures using Images instead of RenderTextures is far better than the current one. If you wanted to isolate that in a separate PR I would merge it immediatlely. Also, I would have to re-read the problems with the crash and the issue in SFML, but its probable that the change would fix it, for what I remember.

I'm gonna open a new pull request just for the texture modification cause It can fix some weird issues and for now I will leave this PR in this state because I think it would need a bit more of a solid base.

[MiguelMJ]

Hi, @JonathSpirit.
As I said in #33 , I've had this a bit abandoned but I plan to revitalize the project.

I do not see how setTextureRect can be implemented here, the texture is bound to every points of the polygon, I think that if we want to customise the texture rect, we have to use shaders.

You are probably right. That's something I was afraid of when I first started working with lighting, and finally the moment has come! The last time I checked the code from the repository of DaiMysha, the shader looked like something feasible to implement in Candle.
However it is true that the code would benefit from a serious cleanup first (damn you, past self).

I'll be updating.