In article <chrishuff-5CABFF.20002302032001@news.povray.org> , Chris 
Huff <chr### [at] maccom>  wrote:

> I think it could be done without too much trouble or code, just parse
> the pattern into a variable and implement a case in Parse_Pattern to
> handle pattern identifiers that would just copy the needed data. I don't
> see anything in the code that will prevent this being tacked on...but if
> this is going to be in 3.5, or even if the relevant code will be cleaned
> up, there isn't much of a reason to mess with it now.

Well, the problem is the way POV-Ray currently creates "classes" and
"objects".  You inherit properties by placing all contents in a big
define and then the appear everywhere and explicit casting is used to
access the fields.  Now, because when writing the code you can check
which pattern is valid in which context, you can of course cast a
pattern structure back to a texture, media or a pigment.  But what
happens if it is just a plain pattern?  Bad luck, you just accessed
unallocated memory!

As I said, one can go ahead and clean all this up and as said I have a
fairly far amount of work done on that (if you want, I can e-mail it to
you), but once all the nice cleanup is done, you still have to test it
because the changes you have to make to implement this in a clean way
are massive, reaching from lighting.cpp to media.cpp.  The real problem
is the testing because you have to clean up the media, pigment and
texture parsing code at the same time - just take a look into
Parse_Pattern...

The good thing about this is that all the ugly casting goes away.  You
even gain some memory by moving data to its appropriate place.  However,
more remains to be done:  The complex question of what constitutes a
"pattern" and what a "pigment".  As you know, for some patterns like
checker you declare two colors along with the patterns while for others
you have to use a color map (referred to as Blend_Map inside the source
code).  Unfortunately, access to it is not unambiguous inside the source
code.  I was a bit surprised about this myself when seeing it the first
time, but in essence the whole pattern structure contains fields that
are misused all over the place For example the "next" pointer is
actually used by media only, and has nothing to do with patterns.  I
have that cleaned up already, but then it just gets worse because this
is what actually brings the media code into the game.

Oh, and of course the patterns that aren't really patterns (like the
"average" pattern, see the docs) had to be fixed.  Take a look at the
function pattern parse code in the current code base and you will know
what I mean.  It works around the problem by simply copying a subset of
the pattern parsing code and for all practical purposes creates a
pattern only object.

Don't get me wrong:  I do not say it is an impossible task, not even a
hard task, but it is an awful lot of work in a lot of parts of the code
if it isn't just supposed to be a quick hack (like so many things we had
to fix in MegaPOV code already).

And it is simply too easy to make some mistake, and such a mistake may
cause crashes only in a strange combination of texture, pigment, media
and normal.  It would be a testing nightmare.  So the premise is to
"never change a running system", especially if it is as essential as
this one!  Nevertheless, for 4.0 this will all be fixed properly
anyway...


    Thorsten


____________________________________________________
Thorsten Froehlich
e-mail: mac### [at] povrayorg

I am a member of the POV-Ray Team.
Visit POV-Ray on the web: http://mac.povray.org

Post a reply to this message