Tessellation again #26

Merged
merged 57 commits into from Feb 4, 2015

Projects

None yet

3 participants

@dardoria
dardoria commented May 9, 2011

Hi,
I've reworked the code that does tessellation applying your comments.
I've also ported the quadrics and the bezcurve examples.

Thanks
Boian

dardoria added some commits Aug 18, 2010
@dardoria dardoria Start of completion of support for tessellation 2256e01
@dardoria dardoria Continue with callbacks definitions. Define a few methods for tessell…
…ator.
f758862
@dardoria dardoria Rename tesselator to tessellator. Fix typos. 3fb00dc
@dardoria dardoria More on tessellation. 5bb2f4a
@dardoria dardoria Register callbacks. Follow the model of glut window events. ff901ad
@dardoria dardoria Continue on tessellation. First drop of example. 51a0313
@dardoria dardoria 179d555
@dardoria dardoria Clean-up names. Continue working on redbook example and bug fixing. f0f3499
@dardoria dardoria Fixes. 44fc82d
@dardoria dardoria Add gitingore e0046b9
@dardoria dardoria More fixes. c6baab5
@dardoria dardoria Declare correct values for tessellation type enum. Clean up tessellat…
…ion callbacks to use the user-data ones. I will probably revisti this but for now it is a good simplificaion.
f394524
@dardoria dardoria Fixes for polygn tessellation 529a936
@dardoria dardoria Start glu-tess-property c4ec5cb
@dardoria dardoria set glu-tess-proeprty 96a815f
@dardoria dardoria tess-combine-data e836d6e
@dardoria dardoria Set colors of the star 5a4ebfb
@dardoria dardoria Add tess-wind example 09de13a
@dardoria dardoria Minor fixes and cleanup e1fb06c
@dardoria dardoria Added delete tess d93ef2c
@dardoria dardoria Apply comments from 3b:
- remove tess prefixes for enums
- use with-new-list
- rename cb to callback
- use enum for winding options instead of constants
310b66a
@dardoria dardoria Close parens. 1da03c4
@dardoria dardoria Add with-tess-polygon and with-tess-contour macros. 99d7071
@dardoria dardoria Free memory allocated with tess-vertex. e343121
@dardoria dardoria Cleanup data created from tess-combine-callback. 74d49d8
@dardoria dardoria Add missing files in asd definitions. Fix number of arguments to meth…
…ods to match the definitions.
4926564
@dardoria dardoria Free data allocated with tess-vertex and tess-callback. e202943
@dardoria dardoria Start rearraning stuff to remove need to deal with cffi in callbacks. ce5fb27
@dardoria dardoria Continue rearranging stuff for easier consumption. Tess-wind example …
…works, tess obly partially.
cc462d2
@dardoria dardoria Few stylistic changes. d687d1e
@dardoria dardoria Use different names for init functions. When both tess and tess-wind …
…were loaded, tess was calling the init function of init-wind which killed glut...
fc50c27
@dardoria dardoria Convert combined vertex-data to array of gl-arrays. d8c6b20
@dardoria dardoria Quadrics example and a function. 972de43
@dardoria dardoria Clear vertex data length of tessellator object. 2323626
@dardoria dardoria Bezcurve example (needs fixing). 6ca2c82
@dardoria dardoria Finish bezcurve example. 7c29fbe
@dardoria dardoria Explicitly declare generic functions for tessellators. 906c247
@dardoria dardoria Restore lines deleted by mistake. 1ddaa8f
@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/bezcurve.lisp
@@ -0,0 +1,66 @@
+;;;; -*- Mode: lisp; indent-tabs-mode: nil -*-
+;;; bezcurve.lisp --- Lisp version of bezcurve.c (Red Book examples)
+;;;
+;;; This program uses evaluators to draw a Bezier curve.
+
+(in-package #:cl-glut-examples)
+
+(defvar control-points (make-array '(4 3) :initial-contents
@3b
3b May 20, 2011 owner

put ** on special variables: *control-points*

@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/bezcurve.lisp
+ (/ (* 5 height) width) -5 5)
+ (gl:ortho (/ (* -5 width) height) (/ (* 5 width) height)
+ -5 5 -5 5))
+ (gl:matrix-mode :modelview)
+ (gl:load-identity))
+
+(defmethod glut:keyboard ((w bezcurve-window) key x y)
+ (declare (ignore x y))
+ (when (eql key #\Esc)
+ (glut:destroy-current-window)))
+
+(defun rb-bezcurve ()
+ (setf glut:*run-main-loop-after-display* nil)
+ (glut:display-window (make-instance 'bezcurve-window))
+ (init-bezcurve)
+ (glut:main-loop))
@3b
3b May 20, 2011 owner

rb-bezcurve should bind glut:*run-main-loop-after-display* locally with LET instead of using SETF so it doesn't affect other code using glut

@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/quadric.lisp
+ (glu:quadric-draw-style quadric-obj :line) ;;all polygons wireframe
+ (glu:quadric-normals quadric-obj :none)
+ (gl:with-new-list ((+ 2 *start-list*) :compile)
+ (glu:disk quadric-obj 0.25 1 20 4))
+
+
+ (glu:quadric-draw-style quadric-obj :silhouette) ;;boundary only
+ (glu:quadric-normals quadric-obj :none)
+ (gl:with-new-list ((+ 3 *start-list*) :compile)
+ (glu:partial-disk quadric-obj 0 1 20 4 0 225))))
+
+(defun rb-quadric ()
+ (setf glut:*run-main-loop-after-display* nil)
+ (glut:display-window (make-instance 'quadric-window))
+ (init-quadric)
+ (glut:main-loop))
@3b
3b May 20, 2011 owner

same issue as rb-bezcurve about binding glut:*run-main-loop-after-display* locally. Probably should also add a local binding of *start-list* so it doesn't conflict with the one in tess.lisp

@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/quadric.lisp
+ (gl:clear-color 0 0 0 0)
+ (gl:material :front :ambient mat-ambient)
+ (gl:material :front :specular mat-specular)
+ (gl:material :front :shininess mat-shininess)
+ (gl:light :light0 :position light-position)
+ (gl:light-model :light-model-ambient model-ambient)
+ (gl:enable :lighting)
+ (gl:enable :light0)
+ (gl:enable :depth-test)
+
+ ;; Create 4 display lists, each with a different quadric object.
+ ;; Different drawing styles and surface normal specifications
+ ;; are demonstrated.
+
+ (setf *start-list* (gl:gen-lists 4))
+ (setf quadric-obj (glu:new-quadric))
@3b
3b May 20, 2011 owner

Should quadric-obj be deleted somewhere? possibly a with-quadric macro that automatically cleans it up would be better here?

@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/tess-wind.lisp
+
+(defmethod glu:combine-data-callback ((tess winding-tessellator) coords vertex-data weight polygon-adata)
+ (loop for i from 0 below 3
+ collect (gl:glaref coords i)))
+
+(defun init-tess-wind ()
+ (gl:clear-color 0 0 0 0)
+ (gl:shade-model :flat)
+ (setf *list* (gl:gen-lists 4))
+ (make-new-lists))
+
+(defun rb-tess-wind ()
+ (setf glut:*run-main-loop-after-display* nil)
+ (glut:display-window (make-instance 'tess-wind-window))
+ (init-tess-wind)
+ (glut:main-loop))
@3b
3b May 20, 2011 owner

same as other examples, bind glut:*r-m-l-a-d* and *list* locally in rb-tess-wind

@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/tess.lisp
+ ;;smooth shaded, self-intersecting star
+ (setf tobj (make-instance 'star-tessellator))
+ (gl:with-new-list ((1+ *start-list*) :compile)
+ (gl:shade-model :smooth)
+ (glu:tess-property tobj :winding-rule :positive)
+ (glu:with-tess-polygon (tobj nil)
+ (glu:with-tess-contour tobj
+ (loop for coords in star
+ do (glu:tess-vertex tobj coords)))))
+ (glu:tess-delete tobj)))
+
+(defun rb-tess ()
+ (setf glut:*run-main-loop-after-display* nil)
+ (glut:display-window (make-instance 'tess-window))
+ (init-tess)
+ (glut:main-loop))
@3b
3b May 20, 2011 owner

same as other examples, bind glut:*r-m-l-a-d* and *start-list* locally

@3b 3b commented on an outdated diff May 20, 2011
glu/interface.lisp
+(defmacro init-tessellation-callbacks (&body callback-specs)
+ `(progn
+ (setq *tess-callbacks* '())
+ ,@(loop for (name callback-type args) in callback-specs
+ collect `(init-tessellation-callback ,name ,callback-type ,args))))
+
+(defmacro with-tess-polygon ((tess-obj polygon-data) &body body)
+ `(unwind-protect (tess-begin-polygon ,tess-obj ,polygon-data)
+ (progn ,@body)
+ (tess-end-polygon ,tess-obj)))
+
+(defmacro with-tess-contour (tess-obj &body body)
+ `(unwind-protect (tess-begin-contour ,tess-obj)
+ (progn ,@body)
+ (tess-end-contour ,tess-obj)))
+
@3b
3b May 20, 2011 owner

shouldn't with-tess-polygon and with-tess-contour be

  `(unwind-protect
        (progn
          (tess-begin-...)
          ,@body)
     (tess-end-...))

?

@3b 3b commented on an outdated diff May 20, 2011
glu/interface.lisp
+
+(defmethod tess-begin-contour ((tess tessellator))
+ (glu-tess-begin-contour (glu-tessellator tess)))
+
+(defmethod tess-vertex ((tess tessellator) coords &optional (vertex-data nil))
+ (let* ((coords-data (list-to-pointer coords))
+ (vertex-data-pointer (if vertex-data
+ (list-to-pointer vertex-data)
+ coords-data)))
+ (glu-tess-vertex (glu-tessellator tess) coords-data vertex-data-pointer)
+ (save-data-to-free coords-data tess)
+ (save-data-to-free vertex-data-pointer tess)
+ (if (and (< 0 (vertex-data-length tess))
+ (not (= (vertex-data-length tess) (length coords))))
+ (warn "Vertex coordinates data must have the same length for one polygon.")
+ (setf (vertex-data-length tess) (length coords)))))
@3b
3b May 20, 2011 owner

shouldn't these be looking at the length of VERTEX-DATA instead of COORDS (at least when VERTEX-DATA is provided)?

@3b 3b commented on an outdated diff May 20, 2011
glu/interface.lisp
+
+;;;; Functions
+(defun register-callbacks (tess)
+ "When creating an object instance check what methods it specializes and regiser appropriate callbacks for each of them."
+ (loop for tess-callback in *tess-callbacks*
+ when (compute-applicable-methods
+ (tess-callback-generic-function tess-callback)
+ (cons tess (loop repeat (tess-callback-arg-count tess-callback) collect t)))
+ do (glu-tess-callback (glu-tessellator tess)
+ (tess-callback-callback-type tess-callback)
+ (get-callback (tess-callback-callback tess-callback)))))
+
+(defun save-data-to-free (data-to-free tess)
+ (when (and (pointerp data-to-free)
+ (not (null-pointer-p data-to-free)))
+ (pushnew data-to-free (data tess) :test 'cffi:pointer-eq)))
@3b
3b May 20, 2011 owner

This PUSHNEW was showing up in profiles when I was testing, might be nice to have an option to just PUSH when the caller knows the pointer was freshly allocated, which I think is true for most of the calls.

@3b 3b commented on an outdated diff May 20, 2011
glu/interface.lisp
+ (when (and (pointerp vertex-data)
+ (not (null-pointer-p vertex-data))
+ (< 0 (vertex-data-length tessellator)))
+ (gl::make-gl-array-from-pointer vertex-data '%gl:double (vertex-data-length tessellator))))
+
+(defun ->combine-vertex-data-array (vertex-data tessellator)
+ (let ((result (cl:make-array 4)))
+ (loop for i from 0 below 4
+ do (setf (aref result i) (->vertex-data-array (mem-aref vertex-data ':pointer i) tessellator)))
+ result))
+
+(defun ->polygon-data-array (polygon-data tessellator)
+ (when (and (pointerp polygon-data)
+ (not (null-pointer-p polygon-data))
+ (< 0 (polygon-data-length tessellator)))
+ (gl::make-gl-array-from-pointer polygon-data '%gl:double (polygon-data-length tessellator))))
@3b
3b May 20, 2011 owner

->polygon-data-array doesn't look right, i don't see anything setting polygon-data-length in the rest of the code, and I don't think anything requires it to be an array of doubles anyway.

@3b 3b commented on an outdated diff May 20, 2011
glu/interface.lisp
+ do (setf (aref result i) (->vertex-data-array (mem-aref vertex-data ':pointer i) tessellator)))
+ result))
+
+(defun ->polygon-data-array (polygon-data tessellator)
+ (when (and (pointerp polygon-data)
+ (not (null-pointer-p polygon-data))
+ (< 0 (polygon-data-length tessellator)))
+ (gl::make-gl-array-from-pointer polygon-data '%gl:double (polygon-data-length tessellator))))
+
+(defun list-to-pointer (list)
+ (when list
+ (let* ((list-length (length list))
+ (pointer (foreign-alloc '%gl:double :count list-length)))
+ (loop for i from 0 below list-length
+ do (setf (mem-aref pointer '%gl:double i)
+ (float (elt list i))))
@3b
3b May 20, 2011 owner

I'd write that loop as

(loop
   for elt in list
   for i from 0
   do (setf (mem-aref pointer '%gl:double i)
            (float elt 1d0)))

calling ELT for every element could get slow if the list is long

Though it might be better to accept any sequence, not just a LIST, in which case it could be better to use MAP instead of LOOP

@3b 3b commented on an outdated diff May 20, 2011
glu/interface.lisp
+ (glu-delete-tess (glu-tessellator tess))
+ (free-tess-data tess))
+
+(defmethod tess-begin-polygon ((tess tessellator) &optional (polygon-data nil))
+ (setf *active-tessellator* tess)
+ (glu-tess-begin-polygon (glu-tessellator tess)
+ (or polygon-data (null-pointer))))
+
+(defmethod tess-begin-contour ((tess tessellator))
+ (glu-tess-begin-contour (glu-tessellator tess)))
+
+(defmethod tess-vertex ((tess tessellator) coords &optional (vertex-data nil))
+ (let* ((coords-data (list-to-pointer coords))
+ (vertex-data-pointer (if vertex-data
+ (list-to-pointer vertex-data)
+ coords-data)))
@3b
3b May 20, 2011 owner

Would probably be nicer if COORDS and VERTEX-DATA could be VECTORs (or sequence in general) in addition to LISTs

Might also be good to support passing a gl:gl-array instead of a lisp sequence.

@3b 3b commented on an outdated diff May 20, 2011
examples/redbook/tess.lisp
+
+(defmethod glu:combine-data-callback ((tess star-tessellator) coords vertex-data weight polygon-data)
+ (let ((vertex '()))
+ (loop for i from 3 downto 0
+ do (push (gl:glaref coords i) vertex))
+
+ (loop for i from 5 downto 0
+ do (push (+ (* (gl:glaref weight 0)
+ (gl:glaref (aref vertex-data 0) i))
+ (* (gl:glaref weight 1)
+ (gl:glaref (aref vertex-data 1) i))
+ (* (gl:glaref weight 2)
+ (gl:glaref (aref vertex-data 2) i))
+ (* (gl:glaref weight 3)
+ (gl:glaref (aref vertex-data 3) i)))
+ vertex))
@3b
3b May 20, 2011 owner

these loops don't look right... first loops 1 too many times, i think, and second should only be looking at elements 3,4,5. If so, i think the loops need to be swapped as well..
Might be simpler (or at least more obvious what is happening) to just pre-allocate storage for the right number of elements and assign them directly rather than looping in reverse with PUSH

@3b
Owner
3b commented May 20, 2011

OK, looks like it is getting closer to done I think...

Main remaining major issues I see are the global *active-tessellator* variable and the handling of the polygon-data.

I think instead of a global for the *active-tessellator* and a raw CFFI pointer for polygon-data, it might be better to store the user data in the tessellator object, and use the polygon-data argument to pass some value the callbacks could use to look up the tessellator object in a global hash table or something.

Might also be interesting to allow users to specify a gl-array type to use with the vertex-data to allow accessing components by name instead of counting indices. (not completely sure about the performance of gl-array stuff yet though, so don't know if i'd want to require it or not)

unknown and others added some commits Apr 25, 2011
unknown changed function names in emit-gl-array-bind-clause for tex-coord, ed…
…ge-flag, and vertex-attrib to ,func-name. (was giving errors for not having %gl:tex-coord)
09206d3
@3b typo fix glut:get-modifers -> glut:get-modifiers, closes #24 514d468
@3b update molview example from hcsw.org/downloads/molview.lisp (new lice…
…nse)
1df3dfe
@3b add enums for more mouse buttons
not sure how many are valid, but my mouse seems to return clicks for
buttons it doesn't even have...
202dd14
@dardoria dardoria Merge remote-tracking branch 'upstream/master' 9c2e0bc
@dardoria dardoria Rename *start-lists* in tess, tess-wind, quadric and bezcurve. Set ru…
…n-main-loop-after-display locally. Apply other changes recommended by 3b.
ac68589
@luismbo

I think you should use window slots for these variables as the other examples do.

Makes sense. I wanted to stay close to the C examples as much as possible that's why I went with the global vars.

@luismbo

This setup doesn't work well with run-all-examples since it'll forcefully run the main loop and thus halt at this example. Why the explicit call too init-bezcurve? Can you use an display-window :before/:after instead?

Same comment here about wanted to be close to the c example. I will change it.

@luismbo

I think you want GLUT:DISPLAY-WINDOW :BEFORE here. GLUT:DISPLAY runs on every iteration of GLUT's main loop.

(This comment applies to the other GLUT:DISPLAY :BEFORE methods.)

@3b
Owner

should be :edge-flag-data instead of :edge-flag

@3b 3b merged commit 4001e8f into 3b:master Feb 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment