Difference between GLSL and BI render when Value node is connected to the Output node #44755

New Issue

Alexander Romanov · 2015-05-18T17:52:47+02:00

Alexander Romanov commented

2015-05-18 17:52:47 +02:00

Steps to reproduce the bug:

Enable Material mode.
Create node material.
Connect Value node to the Output node.

It looks like regression after adding optimizations for set_value and set_rgb functions in gpu_codegen.c:

if (STREQ(name, "set_value") || STREQ(name, "set_rgb"))

Steps to reproduce the bug: 1) Enable Material mode. 2) Create node material. 3) Connect Value node to the Output node. It looks like regression after adding optimizations for set_value and set_rgb functions in gpu_codegen.c: ``` if (STREQ(name, "set_value") || STREQ(name, "set_rgb")) ```

Alexander Romanov commented

2015-05-18 17:52:47 +02:00

Changed status to: 'Open'

Bastien Montagne was assigned by Alexander Romanov

2015-05-18 17:52:47 +02:00

Alexander Romanov commented

2015-05-18 17:52:47 +02:00

Added subscriber: @AlexanderRomanov

Bastien Montagne removed their assignment 2015-05-18 18:03:42 +02:00

Bastien Montagne commented

2015-05-18 18:03:42 +02:00

Added subscriber: @mont29

Bastien Montagne commented

2015-05-18 18:03:42 +02:00

Please do not make wild assignments that way, especially when you do not know what you are doing.

Antonis Ryakiotakis commented

2015-05-18 19:32:03 +02:00

Added subscriber: @Psy-Fi

Antonis Ryakiotakis self-assigned this 2015-05-18 19:32:03 +02:00

Antonis Ryakiotakis commented

2015-05-18 19:32:03 +02:00

Let me take a look

Sergey Sharybin commented

2015-05-18 19:46:46 +02:00

Added subscriber: @Sergey

Antonis Ryakiotakis commented

2015-05-18 20:19:46 +02:00

Added subscriber: @brecht

Antonis Ryakiotakis commented

2015-05-18 20:19:46 +02:00

So, bug exists since a long time. I am not sure exactly what purpose the duplication under those streq in the code is meant to serve, but removing those checks works by accident. Basically, the set_* function output variables get expanded/converted to the right types in codegen_convert_datatype, which is called if the input is of type GPU_SOURCE_TEX_PIXEL. I need to study the code further here to see what is happening, but expanding such variables should be doable in general.

@brecht I would really value your input on why these exceptions exist in the code, I have been wondering for a long time - also we have a few other set functions that might need similar treatment?

So, bug exists since a long time. I am not sure exactly what purpose the duplication under those streq in the code is meant to serve, but removing those checks works by accident. Basically, the set_* function output variables get expanded/converted to the right types in codegen_convert_datatype, which is called if the input is of type GPU_SOURCE_TEX_PIXEL. I need to study the code further here to see what is happening, but expanding such variables should be doable in general. @brecht I would really value your input on why these exceptions exist in the code, I have been wondering for a long time - also we have a few other set functions that might need similar treatment?

Brecht Van Lommel commented

2015-05-18 23:04:52 +02:00

I don't remember this well. From looking at 178be13f65 which added this code, my guess is it's an optimization to leave out unnecessary set_value and set_rgb from the generated code, to make it smaller. I'm not sure that's actually important though and perhaps the simple correct fix is to just leave it out so that the datatype conversion code is used.

There's another similar test in GPU_link_changed which is important to keep, it skips generating code when e.g. diffuse reflectivity has a fixed value of 0.

I don't remember this well. From looking at 178be13f65 which added this code, my guess is it's an optimization to leave out unnecessary `set_value` and `set_rgb` from the generated code, to make it smaller. I'm not sure that's actually important though and perhaps the simple correct fix is to just leave it out so that the datatype conversion code is used. There's another similar test in `GPU_link_changed` which is important to keep, it skips generating code when e.g. diffuse reflectivity has a fixed value of 0.

Antonis Ryakiotakis commented

2015-05-19 15:16:16 +02:00

Hi @brecht, thanks for the heads up!

I studied the code in more detail. Looks like what the optimization does is take the set_value node input link directly (since it's just copying stuff around anyway). The node itself is pruned later. I don't think it's a bad idea generally, so I'll probably just use codegen_convert_datatype to uniforms as well instead. Fingers crossed it works as expected...

Hi @brecht, thanks for the heads up! I studied the code in more detail. Looks like what the optimization does is take the set_value node input link directly (since it's just copying stuff around anyway). The node itself is pruned later. I don't think it's a bad idea generally, so I'll probably just use codegen_convert_datatype to uniforms as well instead. Fingers crossed it works as expected...

blender-admin commented

2015-05-19 15:37:08 +02:00

This issue was referenced by 78411dc7d7

This issue was referenced by 78411dc7d77328e6c8fb7b38f7cacd186e68171e

Antonis Ryakiotakis commented

2015-05-19 15:37:27 +02:00

Changed status from 'Open' to: 'Resolved'

Antonis Ryakiotakis closed this issue

2015-05-19 15:37:27 +02:00

Antonis Ryakiotakis commented

2015-05-19 15:37:49 +02:00

Kept the optimization when input-output types match, it should be fine now, thanks for the report!

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Difference between GLSL and BI render when Value node is connected to the Output node #44755