POV-Ray / povray

The Persistence of Vision Raytracer: http://www.povray.org/
GNU Affero General Public License v3.0
1.35k stars 282 forks source link

Inconsistencies in user bounding and user bounding control. #295

Open wfpokorny opened 7 years ago

wfpokorny commented 7 years ago

The following example, originally from Sean Day, works as expect using -ur but not with the default +ur though the specified bounded_by box meets the "better bounding" tests in parse.cpp. At the core the +ur mode is not making downstream transform adjustments that consider the bbox leading to confusing and inconsistent behavior.

//
// -ur and the translated bounded_by box works as expected.
// With the +ur default the bounded_by box is used if not transformed  
// or it is transformed in the base object. 

#version 3.7;
global_settings { assumed_gamma 1 }
camera
{ location <0.0, 1.5, -8.5>
  look_at <0.0, 1.5, 0.0>
  angle 30
}
light_source { <-4.5, 10.5, -8.0>, rgb 1 }
box { <-10, 0, -10>, <10, 12, 10> hollow pigment { rgb 1 } }

#declare R = 0.07;
#declare Rp = R + 0.001;

union {
    sphere_sweep {
        cubic_spline 6,
        <1, -1>, R,
        <-1, -1>, R,
        <-1, 1>, R,
        <1, 1>, R,
        <1, -1>, R,
        <-1, -1>, R
        bounded_by { box { -<1.5, 0.5, Rp>, <1.5, 0.5, Rp> } }

        texture { pigment { red 1 } }
        interior_texture { pigment { color rgb <1,1,0> } }

//      translate 1.5 * y    // If translate done here things work. bbox set up in parse.cpp.
    }
    sphere { <-0.3, 0.0, 0.0>, 0.05 pigment { blue 1 } }
    translate 1.5 * y        // This should adjust the bounded_by "bbox" too when the original is
                             // not stripped by a volume test in parse.cpp...
                             // The bounded_by box is adjusted by Translate_Object in object.cpp
                             // But unless -ur is used or extra code to look at the bbox added to
                             // SphereSweep::Compute_BBox(), the adjustment is ignored.
}
sphere { <0.0, 1.9, 0.0>, 0.05 pigment { green 1 } }
sphere { <0.0, 1.7, 0.0>, 0.05 pigment { green 1 } }
sphere { <0.0, 1.5, 0.0>, 0.05 pigment { green 1 } }
sphere { <0.0, 1.3, 0.0>, 0.05 pigment { green 1 } }
sphere { <0.0, 1.1, 0.0>, 0.05 pigment { green 1 } }

The core of it looks to be that Parser::Link_To_Frame strips user bounding for everything but CSGUnions, CSGIntersections, CSGMerges, Polys, TrueTypes and Quadrics and so the bounded_by object originally determined as "better" in Parser::Parse_Object_Mods gets stripped and is not available in downstream transforms.

From the user's perspective they have a bounded_by set up that works for an object, later they use this object as some other position and the bounding no longer works. Mostly - if the original objects bounding mechanism creates bounds enclosing the object - this won't be "seen." It will still degrade any performance improvements coming from better user bounding.

There are secondary issue to with the "better" bounded_by determination in that bad bounding can not reliably be fixed by user bounding because it won't always be determined "better."

Basically users cannot count on the bounding they specify.

My leaning presently is to make the user bounding mechanism deterministic. Meaning +ur would strip all user bounding early in the parsing - we not use user bounding anywhere. When running with -ur, we'd use all user bounding exactly as specified. Such a set up would cleanly "fix" the issue discussed above. Yes, some old scenes would break. Especially where users were not getting the bounding they specified previously.

Related: The bounded_by and clipped_by mechanism today allows for >1 object to be used which I did not know until digging into the code. This opens the door to much more complex bounding of objects by sets of bboxes perhaps themselves in some data structure efficiently searched.

c-lipka commented 7 years ago

Is this a 3.7.1-specific issue, or is this also seen with 3.7.0 or even 3.6?

wfpokorny commented 7 years ago

This is not a 3.7.1 specific issue. I have in my head I ran tests back to 3.6, but let me see if I can find my testing directory... Well I didn't find results laying around, but, re-tested and confirmed the issue is seen in 3.6 too.