clipka <ano### [at] anonymousorg> wrote:
> Carlo C. schrieb:
> > clipka <ano### [at] anonymousorg> wrote:
> >> Unless I perfectly goofed it up, "radiosity off" prevents an object from
> >> *receiving* diffuse interreflection (aka radiosity), while
> >> "no_radiosity" prevents an object from *emitting* diffuse interreflection.
>
> (I just checked whether it indeed does what I thought it does after
> having seen the code - it does do right that :-))


Ach komm, das gibt's doch nicht! :-)


> The syntax for "no_radiosity" was chosen to (a) match the megapov
> syntax, and (b) fit into the family of no_shadow, no_reflection and
> no_image keywords, which all basically say, "for the sake of
> [shadow/reflection/image/radiosity] rays, this object does not exist",
> to be consistent there (which was also probably the motivation for the
> megapov syntax).
>
> The "radiosity off" syntax already existed at that time, so it was just
> left unchanged. It's a bit "dangling" I think.


I fully understand the reasons.

Post a reply to this message

clipka <ano### [at] anonymousorg> wrote:
> Unless I perfectly goofed it up, "radiosity off" prevents an object from 
> *receiving* diffuse interreflection (aka radiosity), while 
> "no_radiosity" prevents an object from *emitting* diffuse interreflection.

  It feels quite confusing. How about reusing existing keywords for a
clearer syntax:

radiosity { emission on/off }
radiosity { collect on/off }

-- 
                                                          - Warp

Post a reply to this message

Warp schrieb:
>   It feels quite confusing. How about reusing existing keywords for a
> clearer syntax:
> 
> radiosity { emission on/off }
> radiosity { collect on/off }

How about sticking to the established megapov syntax, hm?

Post a reply to this message

clipka wrote:
> Warp schrieb:
>>   It feels quite confusing. How about reusing existing keywords for a
>> clearer syntax:
>>
>> radiosity { emission on/off }
>> radiosity { collect on/off }
> 
> How about sticking to the established megapov syntax, hm?

Compatibility to MegaPOV should be of *no* concern when porting a patch or 
feature over to official POV-Ray. Consistency and avoiding new keywords when 
reasonable should be the primary conditions for syntax decisions.

	Thorsten, POV-Team

Post a reply to this message

Thorsten Froehlich schrieb:
> Compatibility to MegaPOV should be of *no* concern when porting a patch 
> or feature over to official POV-Ray. Consistency and avoiding new 
> keywords when reasonable should be the primary conditions for syntax 
> decisions.
> 
>     Thorsten, POV-Team

Though I generally consider this a reasonable position, in this 
particular case there was a reason for the MegaPOV patch to use this 
particular syntax and not a different one, touching one of the very 
points mentioned: Consistency, in this case with the other 
"no_something" keywords.

So using any other syntax as that used in MegaPOV would have meant...:

- breaking consistency among the "no_something" family of keywords

- having to invent a new syntax from scratch

- with not much of a precendence case to orient on

- and with the syntax "radiosity off" already being in use for another 
very different feature.

Plus, as already mentioned, sacrificing the opportunity to use a syntax 
already familiar to the users of a very famous POV-Ray patch - which of 
course would not be sufficient alone, but I think it quite well rounds 
off the whole thing.

Post a reply to this message

clipka <ano### [at] anonymousorg> wrote:
> Though I generally consider this a reasonable position, in this 
> particular case there was a reason for the MegaPOV patch to use this 
> particular syntax and not a different one, touching one of the very 
> points mentioned: Consistency, in this case with the other 
> "no_something" keywords.

  Just because "no_something" is used for some features doesn't mean the
syntax is sound for all possible such features.

  Take, for instance, photon mapping. There's no "no_photons" keyword.
There is "photons { pass_through }" and "photons { collect off }", and
for good reasons.

  I see radiosity being more akin to photon mapping than to things like
no_image and no_reflection.

> - breaking consistency among the "no_something" family of keywords

  Photon mapping already "breaks consistency". Except that it doesn't.
Artificially forcing a feature to the same mold as some other feature
is not always a good idea. With photon mapping the different syntax is
justified, and IMO so it is with radiosity.

> - having to invent a new syntax from scratch

  I fail to see how that is a bad thing. If the new syntax is *better*
and easier to understand, it's definitely a *good* thing, not a bad one.

> - with not much of a precendence case to orient on

  Wrong. See photon mapping.

> Plus, as already mentioned, sacrificing the opportunity to use a syntax 
> already familiar to the users of a very famous POV-Ray patch - which of 
> course would not be sufficient alone, but I think it quite well rounds 
> off the whole thing.

  Just because an unofficial patch has made poor choices in syntax doesn't
mean those same poor choices must be replicated in the official version.

-- 
                                                          - Warp

Post a reply to this message

Warp schrieb:
> 
>   I see radiosity being more akin to photon mapping than to things like
> no_image and no_reflection.

Um... no.

Radiosity is actually just cached diffuse reflections, nothing else. 
It's the forward-raytracing component that places photon mapping 
seriously off the grid.

There is no such thing as a "collect on|off" or "emission on|off" in the 
specular (inter-)reflection block, so why should there be such a thing 
for diffuse interreflection?

>   Just because an unofficial patch has made poor choices in syntax doesn't
> mean those same poor choices must be replicated in the official version.

It isn't a poor choice from my seat, for the reasons already explained, 
and in that light I do think that the syntax of a well-established 
unfficial patch should be favored over an otherwise equally good but 
newly invented syntax.

Post a reply to this message

clipka <ano### [at] anonymousorg> wrote:
> Warp schrieb:
> >
> >   I see radiosity being more akin to photon mapping than to things like
> > no_image and no_reflection.
>
> Um... no.
>
> Radiosity is actually just cached diffuse reflections, nothing else.
> It's the forward-raytracing component that places photon mapping
> seriously off the grid.
>
> There is no such thing as a "collect on|off" or "emission on|off" in the
> specular (inter-)reflection block, so why should there be such a thing
> for diffuse interreflection?
>
> >   Just because an unofficial patch has made poor choices in syntax doesn't
> > mean those same poor choices must be replicated in the official version.
>
> It isn't a poor choice from my seat, for the reasons already explained,
> and in that light I do think that the syntax of a well-established
> unfficial patch should be favored over an otherwise equally good but
> newly invented syntax.

this is my irrelevant view...

But photons and radiosity in "global_settings" are already very similar!

Making radiosity syntax more similar to photons (outside "global_settings")
would not be all bad...

I find that the idea of Warp is slightly better that syntax "no_..." for
radiosity.

Post a reply to this message

clipka <ano### [at] anonymousorg> wrote:
> Warp schrieb:
> > 
> >   I see radiosity being more akin to photon mapping than to things like
> > no_image and no_reflection.

> Um... no.

> Radiosity is actually just cached diffuse reflections, nothing else. 
> It's the forward-raytracing component that places photon mapping 
> seriously off the grid.

  It doesn't matter how radiosity is implemented internally. From the syntax
point of view the important thing is how the user perceives it.

> There is no such thing as a "collect on|off" or "emission on|off" in the 
> specular (inter-)reflection block, so why should there be such a thing 
> for diffuse interreflection?

  Because radiosity settings are a whole lot more complicated than some
specular settings and thus deserve their own block, just like photons.

> >   Just because an unofficial patch has made poor choices in syntax doesn't
> > mean those same poor choices must be replicated in the official version.

> It isn't a poor choice from my seat, for the reasons already explained, 
> and in that light I do think that the syntax of a well-established 
> unfficial patch should be favored over an otherwise equally good but 
> newly invented syntax.

  It is a very poor choice. Do you know why? Because it's rigid and hard
to expand.

  If in the future support for new per-object radiosity settings is added,
what are you going to do? Add new keywords to be used in the main object
definition?

  The advantage of using a per-object "radiosity {}" block now is that in
the future it will be much easier to add new features to it.

  "Well-established" means absolutely nothing if it's a poor choice. It makes
no sense to deliberately drag bad choices.

-- 
                                                          - Warp

Post a reply to this message

Warp schrieb:
>> There is no such thing as a "collect on|off" or "emission on|off" in the 
>> specular (inter-)reflection block, so why should there be such a thing 
>> for diffuse interreflection?
> 
>   Because radiosity settings are a whole lot more complicated than some
> specular settings and thus deserve their own block, just like photons.

No, actually for any particular surface or object, the radiosity 
settings are even simpler than the diffuse settings (because they take 
part of the information from there).

In contrast to the photons algorithm, which /needs/ per-object settings 
to be anywhere close to performant, there's virtually only one single 
reason for the per-object or per-material settings currently implemented 
- and that's the same as for the no_reflection, no_shadow or no_image: 
Pure artistic choice.

>   It is a very poor choice. Do you know why? Because it's rigid and hard
> to expand.

There is some point to that - but that choice has been made much, much 
earlier, when "no_shadow", "no_image" and "no_reflection" were 
introduced. Fix those to be more flecible, and I'll happily agree that 
the no_radiosity feature should fall in with them in that more flexible 
syntax.

>   If in the future support for new per-object radiosity settings is added,
> what are you going to do? Add new keywords to be used in the main object
> definition?

There's another difference you're missing here:

no_radiosity does not affect the radiosity illumination calculation for 
/this/ object - it affects the calculations for /other/ objects. (Just 
like no_shadow affects whether a shadow is seen on /other/ objects, and 
no_reflection, too, affects the effective color seen on some /other/ 
object).

If ever the radiosity code would be parameterized on a per-object basis 
(and why should it? shouldn't that be part of the material properties?), 
still the "no_radiosity" object flag wouldn't properly fit in.

Maybe a "visibility {}" block would be a proper container for those flags.

>   The advantage of using a per-object "radiosity {}" block now is that in
> the future it will be much easier to add new features to it.
> 
>   "Well-established" means absolutely nothing if it's a poor choice. It makes
> no sense to deliberately drag bad choices.

It does.

If you were to program an extension module to a spacecraft control 
software natively working with feet and miles, it would be a bad idea to 
try rewrite the whole smash to use metric units. And it would be 
probably just as bad to insist on writing your module to use metric 
units and convert back and forth.

Post a reply to this message