markusfisch / ShaderEditor

Android app to create GLSL shaders and use them as live wallpaper
https://play.google.com/store/apps/details?id=de.markusfisch.android.shadereditor
MIT License
911 stars 136 forks source link

Time uniform accuracy. #14

Closed AntonioND closed 8 years ago

AntonioND commented 9 years ago

Shouldn't time be an integer value? Float values have problems with accuracy after some time, specially 32 bit floats. I know that most times the effect is only visible for a few seconds and accuracy shouldn't be an issue, but this kind of wallpapers are used to show off, so it would be a good idea to make them work fine even if they are active more than a few seconds. Some effects i've tested (like this one: http://glslsandbox.com/e#26016.0) begin to show problems after 20 seconds or so... The problem is the accuracy, not the frame rate. The FPS meter shows the same FPS before and after the problems are noticeable.

Maybe it would be a better idea to add a "frame count" uniform.

I'm using a Motorola Moto G 2, Android 5.0.1

markusfisch commented 9 years ago

Well, yes, probably. I used a float because the time almost certainly ends up in a calculation with floats anyway and this way there is no further conversation necessary.

On the other hand, the GPU will probably auto convert ints into floats so that may not make much of a difference. I will have to make some tests to see if there are any side effects from using an int; especially on bad/old hardware. Don't want to break anything here.

Of course, another way to avoid accuracy problems would be to use high precision floats:

precision highp float;

AntonioND commented 9 years ago

You don't have to remove the old uniform, just add a new one. The problem is that high precision floats are slower, and smartphones aren't very powerful... With an integer you can trim the integer value before converting it into a float, so you don't lose accuracy.

markusfisch commented 9 years ago

That's right, of course. I'd just prefer to not having two time uniforms when one would do. So if it doesn't have any side effects, I'll convert the existing one, but if it does, I'll simply add a new one like you suggested.

And, indeed, highp can be anything from unavailable to low performance on smartphones, unfortunately. At least, it slowly gets better with new devices I think.

AntonioND commented 9 years ago

Oh, and highp doesn't solve the problem because the time data is converted to float before calling the shader, so the most you can get is 32 bit accuracy, right?

markusfisch commented 9 years ago

Well, that depends on the problem. highp does solve some precision problems on some devices.

But in general, you're right, the time is converted to a 32 bit float before handing it to the shader.

Tetane commented 9 years ago

Hi! I think I have the same issue but I'm not sure. Over time, the time variable become slower and slower but the number of FPS is still the same and the UI and the touch are not affected. If I lock and unlock my phone, it reset the problem so the time variable is fast again but it is still getting slower and slower over time.

Hope it helps! ( Awesome app to learn GLSL by the way ;) )

Edit: My phone is a Samsung Galaxy note 2 (gt-n7100)

markusfisch commented 9 years ago

Very strange. Honestly I don't have an idea yet what's causing this but it's certainly interesting. Thanks a lot for reporting!

AntonioND commented 9 years ago

It could be the same issue. When the value of the variable is big the increments are less noticeable so it may result in that kind of behaviour.

markusfisch commented 9 years ago

Yes, you're right, of course! Really hope I can do an update very soon. That really needs to be fixed.

AntonioND commented 9 years ago

Yeah, it's a shame that this problem is there because the rest of the program is really good. This one and the other active issue of not knowing when the screen is pressed or not.

Tetane commented 9 years ago

Hello again ! The issue doesn't seem to come from your app!!! I'm programming fragment shaders in GLSL with LibGDX and I have exactly the same issue on android. LibGDX is a multi-platform library in java, and when running the shader on PC it works fine. The issue only appear on android.

Tested on my two android devices : Samsung Galaxy note 2 (gt-n7100), and Archos GamePad 2.

markusfisch commented 9 years ago

Hi, I think I found a solution for this. Check this commit.

The root problem is a GLSL float is not very long (especially with mediump) and you get very quickly rounding errors with large numbers.

So I simply wrap time before the rounding errors get noticeable. What should be perfectly okay because time is basically nothing but a marker for where you are in an repeating animation.

fekga commented 9 years ago

Hi! I was thinking about this time problem and I think the best solution would be to send an integer and a float to the shader, the integer representing the seconds and the float the fractional part of a second. This way anyone could wrap themself the time and it wont create a gap in animation. Also you could then just add the two values together to get the actual time value. On 17 Aug 2015 12:00, "Markus Fisch" notifications@github.com wrote:

Hi, I think I found a solution for this. Check this commit https://github.com/markusfisch/ShaderEditor/commit/0056b931636b8d061c751bc49bd0bc0ab4391118 .

The root problem is a GLSL float is not very long (especially with mediump) and you get very quickly rounding errors http://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html#680 with large numbers.

So I simply wrap time before the rounding errors get noticeable. What should be perfectly okay because time is basically nothing but a marker for where you are in an repeating animation.

— Reply to this email directly or view it on GitHub https://github.com/markusfisch/ShaderEditor/issues/14#issuecomment-131761165 .

markusfisch commented 9 years ago

No, this wouldn't work.

First, according to the specification, a mediump int has a range from -2^10 to 2^10 (-1024 to 1024). What means it would wrap just after a second.

And mediump is still important since the app should also run on low end devices.

Then, if we'd use an int, we would almost certainly have to cast it in the shader what is at least an unnecessary inconvenience.

fekga commented 9 years ago

You're right, sorry. I wasn't aware of the precision specifiers' specifications. But a wrap will still create a jump in animation, so the one that creates the shader should be aware of the limitations of his device and overcome them by wrapping the values themselves. On 17 Aug 2015 21:43, "Markus Fisch" notifications@github.com wrote:

No, this wouldn't work.

First, according to the specification https://www.khronos.org/registry/gles/specs/2.0/GLSL_ES_Specification_1.0.17.pdf, a mediump int has a range from -2^10 to 2^10 (-1024 to 1024). What means it would wrap just after a second.

And mediump is still important since the app should also run on low end devices.

Then, if we'd use an int, we would almost certainly have to cast it in the shader what is at least an unnecessary inconvenience.

— Reply to this email directly or view it on GitHub https://github.com/markusfisch/ShaderEditor/issues/14#issuecomment-131939905 .

markusfisch commented 9 years ago

Well, that would make exchanging shaders between devices even more problematic. And it would raise the bar for newcomers as they would have to deal with the hardware limitations of their device first.

So, in my opinion, a well-known, pre-defined limit is probably better. Only that it's not well-known at the moment :) The app really needs a little documentation - within the app - about all available uniforms and their limitations I think.

Would you consider 65535 to be a fair choice for everyone or would you rather choose to have a configurable preference for that? A preference would affect exchangeability, of course, but one would have more freedom.

fekga commented 9 years ago

Yeah I guess 18 hours of time would be enough ;) On 18 Aug 2015 13:53, "Markus Fisch" notifications@github.com wrote:

Well, that would make exchanging shaders between devices even more problematic. And it would raise the bar for newcomers as they would have to deal with the hardware limitations of their device first.

So, in my opinion, a well-known, pre-defined limit is probably better. Only that it's not well-known at the moment :) The app really needs a little documentation - within the app - about all available uniforms and their limitations I think.

Would you consider 65535 to be a fair choice for everyone or would you rather choose to have a configurable preference for that? A preference would affect exchangeability, of course, but one would have more freedom.

— Reply to this email directly or view it on GitHub https://github.com/markusfisch/ShaderEditor/issues/14#issuecomment-132185541 .

markusfisch commented 9 years ago

No, time is in milliseconds, so 65535 is just over a minute.

The thing is, a mediump float is only 10 bits long too.

fekga commented 9 years ago

As suggested above a second variable could count the minutes passed and time would then wrap around the minute. On 18 Aug 2015 18:39, "Markus Fisch" notifications@github.com wrote:

No, time is in milliseconds, so 65535 is just over a minute.

The thing is, a mediump float is only 10 bits https://www.khronos.org/files/opengles_shading_language.pdf long too.

— Reply to this email directly or view it on GitHub https://github.com/markusfisch/ShaderEditor/issues/14#issuecomment-132270281 .

markusfisch commented 8 years ago

I'm an idiot. time is not in milliseconds but seconds. So 65535 is indeed well over 18 hours.

Fixed in version > 2.

rafaellago commented 8 years ago

Shaders are looking silky smooth after this update.

AntonioND commented 8 years ago

Time is in seconds but you usually need milisecond accuracy for the animation to be smooth, so you need all decimal bits up to a milisecond to be accurate.