Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

With-pushed-matrix doesn't use unwind-protect #33

Open
smithzvk opened this Issue · 4 comments

2 participants

@smithzvk

With-pushed-matrix* does. What are the issues with making with-pushed-matrix behave sanely w.r.t. non-local exits? I assume that this is a deep issue since the fix is so simple and has already been implemented in With-pushed-matrix*?

As a test case:

(dotimes (i 33) (ignore-errors (with-pushed-matrix (error "Uh-oh"))))

might break conforming OpenGL programs. I'm not an OpenGL guru, am I missing something?

@3b
Owner
3b commented

I think the reason I added with-pushed-matrix* instead of adding unwind-protect to with-pushed-matrix was due to the possibility of the current matrix changing before the non-local exit, in which case we pop the wrong matrix, and leave something on the original matrix stack anyway.

Not sure if that is actually enough of a concern to avoid handling the normal case better though...

@3b 3b closed this
@3b 3b reopened this
@smithzvk

Is it ever ok (i.e. not an eventual matrix stack overflow) to not pop a matrix that has been pushed? Perhaps I misunderstand push and pop matrix. My understanding:

(init_matrix) ;; matrix stack (), current matrix is A
(push-matrix) ;; matrix stack (A) and current matrix is A
;; Do anything other than push or pop.  Matrix stack is still (A), current matrix is B where in general B /= A
(push-matrix) ;; matrix stack (B A) and current matrix is B
;; Do anything other than push or pop.  Matrix stack is still (B A), current matrix is C /= B /= A
(pop-matrix) ;; matrix stack (A), current matrix B
;; Do anything other than push or pop.  Matrix stack is still (A), current matrix is D /= A
(pop-matrix) ;; matrix stack is () and current matrix is A

This is how OpenGL works, right? If there is ever an unbalanced push-matrix, and it is in code that is called often, there will eventually be a stack overflow.

@3b
Owner
3b commented

Right, push without pop is bad, but we can't (efficiently) be sure we are popping the right matrix stack, which is why with-pushed-matrix* requires explicitly specifying which matrix stack to push/pop.

(gl:matrix-mode :modelview)
(gl:with-pushed-matrix  ;;; pushes the current matrix stack, 'modelview' in this case
  (gl:matrix-mode :projection)
  (error "broken")
  (gl:matrix-mode :modelview) ;; would set 'current' matrix mode back, but error skips that
  ) ;;; unwind-protect would pop current matrix stack, 'projection' in this case

In this case, even with unwind-protect we still eventually get an overflow on the modelview stack, and we probably get an underflow on the projection matrix stack (or at least the wrong projection matrix for following code)

@smithzvk

Oh, right, good point. In light of this, I change my complaint to: "Really just seems like with-pushed-matrix* is the right default tool for the job, maybe it should be the unstarred version."

I suppose you could do something efficient by having gl:matrix-mode set a special variable that holds what matrix is active, but it is a bit messy. Or you could limit the damage by promoting usage of a gl:with-matrix-mode macro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.