MiguelMJ / Candle

2D lighting for SFML
http://www.miguelmj.dev/Candle/
MIT License
130 stars 8 forks source link

Directed light not casting shadow at 90º #27

Closed MiguelMJ closed 2 years ago

MiguelMJ commented 2 years ago

With the last fixes, now directed lights don't cast shadows when they have a 90º rotation. I'm not sure whether it has to do with the line intersection calculation, but this surely didn't happen before.

Capture 1 ![candle-capture-211018153635](https://user-images.githubusercontent.com/37369782/137742640-24a6317a-76a8-45a7-a6f2-fa7ee8c2c6c9.png)
Capture 2 ![candle-capture-211018154104](https://user-images.githubusercontent.com/37369782/137743066-0ac56d44-c04f-4ef6-8b12-9bc6dfd58e91.png)

@JonathSpirit just mentioning you in case you want to be aware.

JonathSpirit commented 2 years ago

The goal of having 2 equation is to avoid the "0 problem" (like divided by zero).

The check that I do here by checking the angle seems to not be good :

lineAngle > 180.0f

But by doing what was made before a little bit remixed seams to do the trick :

(std::abs(lineAdirection.y) < 0.001f) || (std::abs(lineAdirection.x) > 0.0f)

mmmh

sorry for the inconvenience, math is hard :)

MiguelMJ commented 2 years ago

Absolutely no problem! Thanks again ;)

JonathSpirit commented 2 years ago

I don't know if this was the case before but when the width of the directed light is big glitch appear :

image

JonathSpirit commented 2 years ago

Confirm that this wasn't present before, but this can fixed by doing :

std::abs(lineAdirection.x) >= 0.0f

instead of :

std::abs(lineAdirection.x) > 0.0f
JonathSpirit commented 2 years ago

now the snap reappear x) :

image

MiguelMJ commented 2 years ago

Until now more or less I could understand the errors and solutions... but this really got me. I guess this has to do with floating point precision... But someting tells me that is easier to solve the glitch of DirectedLight than the snap.

JonathSpirit commented 2 years ago

Well I think I found something that fix it all 😪 :

if ( (std::abs(lineBdirection.y) >= 0.0f) && (std::abs(lineBdirection.x) < 0.001f) || (std::abs(lineAdirection.y) < 0.001f) && (std::abs(lineAdirection.x) >= 0.0f) )
{
    normB = (lineAdirection.x*(lineAorigin.y-lineBorigin.y) + lineAdirection.y*(lineBorigin.x-lineAorigin.x))/(lineBdirection.y*lineAdirection.x - lineBdirection.x*lineAdirection.y);
    normA = (lineBorigin.x+lineBdirection.x*normB-lineAorigin.x)/lineAdirection.x;
}
else
{
    normA = (lineBdirection.x*(lineBorigin.y-lineAorigin.y) + lineBdirection.y*(lineAorigin.x-lineBorigin.x))/(lineAdirection.y*lineBdirection.x - lineAdirection.x*lineBdirection.y);
    normB = (lineAorigin.x+lineAdirection.x*normA-lineBorigin.x)/lineBdirection.x;
}

mmmh3

If someone have a better/more performant way to do this cause I'm starting to get lost on that :)

MiguelMJ commented 2 years ago

I'm almost sure it has to do with compiler differences, because although I just forked your own branch for 90fix to see if I got any of the glitches shown in the gifs of this thread, none of them happen to me.. Let's keep this change, as it fixes the glitch for these cases. I'm no expert in high performance applications, but I don't think that the additional checks of this fix will make a noticeable difference in the performance. If they did... well, I guess we could add a compiler flag to choose what check to make 🤷🏼‍♂️ But for now I say it can stay this way.

MiguelMJ commented 2 years ago

Closed in #28