POV-Ray

Identified... it's worst than quick_color.

The creator of Trace (render/trace.cpp) is called twice per thread.
First with the right value for the quality, and it store/update the TraceThreadData, by the creator of TracePixel (called via trace in TraceTask creator).
Then Radiosity code calls it too (creator of RadiosityFunction, in lighting/radiosity.cpp) with a filter via GetRadiosityQualityFlags(), using a hard coded QUALITY_9.
RadiosityQualityFlags remove Q_AREA_LIGHT and if there is no media, remove Q_VOLUME

I believe the creator of Trace must NOT touch the TraceThreadData's member... (object isolation anyone ?) There is already a TODO at that line in Trace::Trace.
But I'm weak. My proposal so far is to create the Data with 0 (instead of QUALITY_9), and to have it updated in Trace's creator only if it is still 0. (so only the first time).

Change #5441, to be inspected and reported if you (code maintainers) agree.
(tested on my branch: it works (but I'm not happy with it))

Edition: I wrote "creator"... it's "constructor". Hopefully you get it.

As a quick fix, I suggest to move the updating of TraceThreadData's qualityFlags into the TraceTask constructor, as the TODO comment already suggests.

For a later "clean" fix, I'd like to note the following design and implementation flaws that play a role here:

- The TraceThreadData object's original purpose is (as far as I can see) to hold thread-specific data; qualityFlags shouldn't be in there in the first place, as it is a thread-independent "constant". The only reason for it being in there is that Compute_Pigment needs to know whether the quick color feature is active or not. It would be cleaner to place it in the SceneData object, which can be accessed via the TraceThreadData object. In case that is not an option (as it may slow down render a bit), the second cleanest solution would be to supply this information at TraceThreadData creation time.

- The RadiosityFunction constructor should be fixed to use a proper quality flags value instead of QUALITY_9, with the same quick color setting as used otherwise.

- There is no need for a RadiosityFunction to be created in the first place unless radiosity is enabled (which it never is in quick-color mode); a RadiosityFunction object is always created though. (The RadiosityFunctor class definition indicates that there might originally have been the intention to use it as a dummy RadiosityFunction object in case radiosity is disabled.)

BTW, the TraceThreadData object seems to be designed primarily as a data container, so strict object isolation isn't intended. The View object does a lot of fiddling with TraceThreadData's members, for instance.

Small comments:
1. I was not requesting object isolation per itself, but I was puzzled by a constructor of an object which updates another in a non obvious way (such as adding itself to a list or set... that is understandable. The 3.7RC3 behaviour of the code is strange)
2. I'm used to Aviation/Nuclear state/Military developping/investigation way of thinking: finding the issue is far more important than finding a guilty coder (In fact, I have probably written worse code than anyone else)
3. I have no idea how to make the clean fix (for the time being ?).

Should now be fixed with #5443 (the quick way)

Should now be fixed with #5443 (the quick way)

Did you leave the TODO in the code so we can fix it completely in the future?

I see no TODO to put in code (at least near the patched sections).

Maybe Christoph has a better idea where to put such comments ?

TODO comment added back in (at new code location) with change #5482