Re: GLU.Tessellator crashes

Balazs Komuves <[email protected]>
Newsgroups gmane.comp.lang.haskell.hopengl
Message-ID <[email protected]>
Hi,

I found the cause of the crashes. A patch which should solve the problem
(but changes the API, see below) is attached.

The problem is that the GLU documentation and the GLU implementation
is inconsistent. The GLU documentation claims that:


> The "combine" or "combineData" callback is invoked to create a
> new vertex when the algorithm detects an intersection, or wishes
> to merge features.  The vertex is defined as a linear combination
> of up to 4 existing vertices, referenced by data[0..3].  The
> coefficients of the linear combination are given by weight[0..3];
> these weights always sum to 1.0.  All vertex pointers are valid
> even when some of the weights are zero.
>

(emphasis on the last sentence); but the GLU implementation
sets some of those pointers to 0:

static void SpliceMergeVertices( GLUtesselator *tess, GLUhalfEdge *e1,
>                  GLUhalfEdge *e2 )
> /*
>  * Two vertices with idential coordinates are combined into one.
>  * e1->Org is kept, while e2->Org is discarded.
>  */
> {
>   void *data[4] = { NULL, NULL, NULL, NULL };
>   GLfloat weights[4] = { 0.5, 0.5, 0.0, 0.0 };
>
>   data[0] = e1->Org->data;
>   data[1] = e2->Org->data;
>   CallCombine( tess, e1->Org, data, weights, FALSE );
>   if ( !__gl_meshSplice( e1, e2 ) ) longjmp(tess->env,1);
> }


(this is from the SGI GLU code, libtess/sweep.c). By the way, the
the example combiner function in the documentation should crash, too.

Since the vertex annotations are of arbitrary type, there is no default
value we could supply when GLU gives us zero pointers; thus I changed
the type WeightedProperties to

data WeightedProperties v =
>   WeightedProperties
>     (GLclampf, v)
>     (GLclampf, v)
>     (Maybe (GLclampf, v))
>     (Maybe (GLclampf, v))
>

Maybe some other solution, like WeightedProperties2 ... |
WeightedProperties4 ...
would be better...

Balazs

On Wed, Mar 11, 2009 at 7:15 PM, Jules Bean <[email protected]> wrote:

> Hi,
>
> I've just written some code which uses 'tessellate' from
> Graphics.Rendering.OpenGL.GLU.Tessellation and whilst it works exactly as
> expected on simple data sets, it gives a bus error on larger ones (not that
> large - e.g. 150 points).
>
> This might be a bug in my OS's glu implementation, but I'm also inclined to
> suspect a bug in all the peeks and pokes HOpenGL uses to handle the data for
> the callbacks. I don't actually use the callback data (I've set all the data
> to (0 :: Int) only because there is no storable instance for ()).
>
> Does anyone have a success story with this function? Has anyone used it
> successfully on larger polygons?
>
> Jules
> _______________________________________________
> HOpenGL mailing list
> [email protected]
> http://www.haskell.org/mailman/listinfo/hopengl
>

_______________________________________________
HOpenGL mailing list
[email protected]
http://www.haskell.org/mailman/listinfo/hopengl
tessel.patch (application/octet-stream, 7 KB)
Fri Mar 13 00:11:53 CET 2009  Balazs Komuves <[email protected]>
  * glu / tessellation / combiner-callback bugfix (the bug  is in glu, not in hopengl); note that the api changes too

New patches:

[glu / tessellation / combiner-callback bugfix (the bug  is in glu, not in hopengl); note that the api changes too
Balazs Komuves <[email protected]>**20090312231153] {
hunk ./Graphics/Rendering/OpenGL/GLU/Tessellation.hs 40
 
 import Control.Monad ( foldM, unless )
 import Data.IORef ( newIORef, readIORef, writeIORef, modifyIORef )
+import Data.Maybe ( fromJust )
 import Foreign.Marshal.Alloc ( allocaBytes )
 import Foreign.Marshal.Array ( peekArray, pokeArray )
 import Foreign.Marshal.Pool ( Pool, withPool, pooledNew )
hunk ./Graphics/Rendering/OpenGL/GLU/Tessellation.hs 240
 data WeightedProperties v
    = WeightedProperties (GLclampf, v)
                         (GLclampf, v)
-                        (GLclampf, v)
-                        (GLclampf, v)
+                        (Maybe (GLclampf, v))
+                        (Maybe (GLclampf, v))
 #ifdef __HADDOCK__
 -- Help Haddock a bit, because it doesn't do any instance inference.
 instance Eq v => Eq (WeightedProperties v)
hunk ./Graphics/Rendering/OpenGL/GLU/Tessellation.hs 648
    newVertex <- peek newVertexPtr
    [v0, v1, v2, v3] <- mapM (getProperty propertyPtrs) [0..3]
    [w0, w1, w2, w3] <- peekArray 4 weights
-   let wp = WeightedProperties (w0,v0) (w1,v1) (w2,v2) (w3,v3)
+   let f w mv = case mv of { Just v -> Just (w,v) ; Nothing -> Nothing } 
+       wp = WeightedProperties (w0,fromJust v0) (w1,fromJust v1) (f w2 v2) (f w3 v3)
        av = AnnotatedVertex newVertex (combiner newVertex wp)
    poke result =<< pooledNew pool av
 
hunk ./Graphics/Rendering/OpenGL/GLU/Tessellation.hs 653
-getProperty :: Storable v => Ptr (Ptr (AnnotatedVertex v)) -> Int -> IO v
+getProperty :: Storable v => Ptr (Ptr (AnnotatedVertex v)) -> Int -> IO (Maybe v)
 getProperty propertyPtrs n = do
hunk ./Graphics/Rendering/OpenGL/GLU/Tessellation.hs 655
-   AnnotatedVertex _ v <- peek =<< peekElemOff propertyPtrs n
-   return v
+   p <- peekElemOff propertyPtrs n
+   if p == nullPtr 
+     then return Nothing
+     else do
+       AnnotatedVertex _ v <- peek p 
+       return (Just v) 
 
 foreign import CALLCONV "wrapper" makeCombineCallback ::
    CombineCallback v -> IO (FunPtr (CombineCallback v))
}

Context:

[TAG GHC 6.8.1 release
Ian Lynagh <[email protected]>**20071110011104] 
[TAG 2.2.1.1 release
Ian Lynagh <[email protected]>**20071110010954] 
[Bump version number
Ian Lynagh <[email protected]>**20071027124805] 
[Specify build-type: Configure
Duncan Coutts <[email protected]>**20071018173137] 
[--configure-option and --ghc-option are now provided by Cabal
Ross Paterson <[email protected]>**20070604115936] 
[Remove Makefile and package.conf.in (used in the old GHC build system)
Ian Lynagh <[email protected]>**20070524145614] 
[add includes: field
Simon Marlow <[email protected]>**20070517094912] 
[Make configure fail if the package cannot be built
Ian Lynagh <[email protected]>**20070430111732] 
[TAG GHC 6.6.1 release
Ian Lynagh <[email protected]>**20070428195851] 
[TAG 2.2.1 release
Ian Lynagh <[email protected]>**20070428195655] 
[TAG GHC 6.6.1 release
Ian Lynagh <[email protected]>**20070424113929] 
[TAG Version 2.2.1
Ian Lynagh <[email protected]>**20070424113831] 
[Bump version to 2.2.1
Ian Lynagh <[email protected]>**20070422195057] 
[Follow Cabal changes in Setup.*hs
Ian Lynagh <[email protected]>**20070418121330] 
[Fix -Wall warnings
Ian Lynagh <[email protected]>**20070411012221] 
[Make Setup.hs suitable for building in a GHC tree
Ian Lynagh <[email protected]>**20070407174116] 
[Importing hs_OpenGL_getProcAddres must use ccall instead of CALLCONV
[email protected]**20070313174609
 
 Patch provided by [email protected]
] 
[README about building from darcs
Ross Paterson <[email protected]>**20070218110200] 
[Note that we support OpenGL 2.1 now
[email protected]**20070101125021] 
[Completed implementation of drawBuffers
[email protected]**20061130133951] 
[Made drawBuffers a full-blown StateVar (getter not yet implemented)
[email protected]**20061130132401] 
[Refactored (un)marshalling of modelview indices, fixing an off-by-two bug on the way
[email protected]**20061130132135] 
[Changed the type of a few implementation limits to GLsizei for consistency reasons
[email protected]**20061130123956] 
[Added current raster secondary color query
[email protected]**20061129125402] 
[Added sRGB textures
[email protected]**20061129121705] 
[Added pixel buffer objects
[email protected]**20061129093054] 
[Made 'uniform' a 'StateVar' to allow queries, too
[email protected]**20061122130106] 
[Added missing Storable instance for FogCoord1
[email protected]**20061122123656] 
[Added vertexProgramTwoSide and vertexProgramPointSize
[email protected]**20061121120200] 
[Do not return trailing NUL in shader string queries
[email protected]**20061115151908] 
[Make Eq/Ord/Show/ObjectName superclasses of Shader
[email protected]**20061112140430] 
[Added utility function 'majorMinor' to easily parse OpenGL's version strings
[email protected]**20061112131225] 
[Extended the set of possible types for vertex attributes and uniform variables
[email protected]**20061110131951] 
[Distributed shader-related stuff to the right modules
[email protected]**20061110122648] 
[Updated Haddock module headers (now OpenGL 2.1 support, API is stable)
[email protected]**20061110101454] 
[Partial implementation of vertex attributes/uniform variables
[email protected]**20061109143842] 
[Added a few missing OpenGL 2.0 features and prepared GLSL variable access
[email protected]**20061108144500] 
[Added Haddock comments for GLSL implementation limits
[email protected]**20061107140514] 
[Completed handling of shaders and programs
[email protected]**20061106151311] 
[Implement almost all shader/program related API entries
[email protected]**20061103141002] 
[More GLSL functionality, mainly queries plus some cleanup
[email protected]**20061102134107] 
[Added shading-related queries
[email protected]**20061102133821] 
[Added missing import
[email protected]**20061102094212] 
[Added basic Shader/Program functionality
[email protected]**20061101210232] 
[First steps towards GLSL support
[email protected]**20061101193753] 
[Added autoconf magic for GLchar/GLintptr/GLsizeiptr
[email protected]**20061101183122] 
[Slightly improved the NURBS API
[email protected]**20061012114037] 
[includes -> install-includes
Ross Paterson <[email protected]>**20060829123744] 
[exclude Setup.hs even if not building package
Ross Paterson <[email protected]>**20060825222701] 
[exclude Setup.hs from build
Ross Paterson <[email protected]>**20060824183533] 
[add boilerplate Setup.hs
Ross Paterson <[email protected]>**20060824115102] 
[add files used by configure
Ross Paterson <[email protected]>**20060518174356] 
[TAG Initial conversion from CVS complete
John Goerzen <[email protected]>**20060112154136] 
Patch bundle hash:
33e10f93119e76b02b752b6df35de7e34c976a0d
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.