Incorrect focal blur on points (with patch)

writhe
2007-12-16
2013-04-25
  • writhe

    writhe - 2007-12-16

    Points which are blurred due to depth of field appear to be "clamped" as in this example:
        http://writhe.org.uk/pixie/test_point_blur_incorrect.png
        http://writhe.org.uk/pixie/test_point_blur.rib

    This seems to be due to the bounds of individual points not being correctly calculated in CReyes::insertGrid when there is focal blur (i.e. non-zero aperture). In particular, the bounds aren't expanded by the "circle of confusion" amount for each individual point, so the bounds still cover the original area (leading to the edges of the point being cut off).

    The fix is relatively straight forward: add the circle of confusion to each point's bounds (similarly to how it's done for quads later in the same function).

    Here's a patch (against revision 1129 of the SVN trunk) and the same scene rendered with the patch:
        http://writhe.org.uk/pixie/point_focal_blur.diff
        http://writhe.org.uk/pixie/test_point_blur_correct.png

    Also, could someone clarify what is stored in CRasterGrid::vertices (as this doesn't seem to be explicitly mentioned anywhere)?

    As I understand it, the first 3 values always hold the X, Y and Z coordinates. The 10th value (i.e. "vertices[9]") is used for the circle of confusion amount measured in samples (assuming depth of field is active in the scene).

    After the first 10 values there are also CRenderer::numExtraSamples for additional colour channels. Is that right?

    Are the other values (in the first 10) used for anything at the moment?

     
    • George Harker

      George Harker - 2007-12-16

      Hi Writhe,

      You're quite right.  Thanks for the fix.

      The vertices' format is probably going to be revised at some point, but currently it stores:

      xyz (3 float)
      color (3 floats)
      opacity (3 floats, needed to composite correctly)
      coc
      ... extra samples for AOVs if any ...

      All 10 are used, and are necessary.  The 10th could be discarded in a non DOF context (I was planning to do this).

      These correspond to the  P,Ci,and Oi outputs from shading (plus and AOVs).

      Cheers

      George

       
    • George Harker

      George Harker - 2007-12-17

      Incidentally, in the ideal case Oi and Ci should not be needed for unshaded grids.  There's a chunk of code I have here which reallocates a fresh grid for shaded grids. This reduces memory overhead of unshaded grids - but it's a little tricky to make it work with multithreading, which is why it's not in there right now.

      Cheers

      George

       
      • writhe

        writhe - 2007-12-17

        Maybe you've already considered this but I think it would be better to break the vertex data into separate arrays instead of the current ‘interleaved’ format.

        This might make it easier for multithreading as you can then copy / swap pointers for the appropriate data (pointer swap can be done as an atomic operation).

        There would also be benefits for caching and hardware prefetching. For example, in the current unshaded grid code, when you load the XYZ data the processor also has to load the rest of the cache line which contains data you don't want (colour, opacity, COC etc.).

        If / when any of this code gets vectorised (SSE / Altivec) it may also be better to further break these into individual component arrays (i.e. separate arrays for X, Y, Z etc.). This avoids having to ‘swizzle’ the data once it's in the SIMD registers.

         

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks