[gsharp-devel] Problem of second gsharp

[Robert STRANDH]

Christophe Rhodes writes:
 > OK, here's take II.  It's slightly uglier than before (but is less
 > broken :-), and I have two questions.
 > 
 > Firstly, what does the comment in the two REPLAY-OUTPUT-RECORD methods
 > that I've touched mean?  ("we replay with the identity transformation,
 > so we have to draw the other way").

It explains why in the replay of upward-beam-output-record there is a
call to draw-downward-beam and vice versa.  The function draw-upward
beam is called when the transformation is such that the origin is in
the lower-left corner, whereas the replay is done with the identity
transformation, and thus with the origin in the upper-left corner. 

 > Secondly, is the change that I've made here (specializing
 > REPLAY-OUTPUT-RECORD on the stream argument, too) guaranteed to work?
 > How will it interfere, if at all, with the notion of replaying all
 > output records on a postscript medium for printing?

I think what you did is right.  In fact, the other draw-xx functions
should probably be specialized as well, so as to allow for other types
of output media, like a PostScript file. 

 > (This fix does improve thread-safety in the sense that two separate
 > gsharps running in the same image would not have collisions in their
 > {dark,light}er-gray-progressions pixmaps.  There remains the problem
 > of multiple threads running the same gsharp -- say for concurrent
 > updating of the screen, or something -- colliding when extending the
 > arrays containing the pixmaps).

OK, that is fine for now. 

 > ===================================================================
 > RCS file: /project/gsharp/cvsroot/gsharp/score-pane.lisp,v
 > retrieving revision 1.1.1.1
 > diff -u -r1.1.1.1 score-pane.lisp
 > --- score-pane.lisp	16 Feb 2004 15:46:21 -0000	1.1.1.1
 > +++ score-pane.lisp	25 Feb 2004 17:39:07 -0000
 > @@ -1,7 +1,11 @@
 >  (in-package :score-pane)
 >  
 >  (defclass score-pane (application-pane)
 > -  ((pixmaps :initform (make-hash-table :test #'eq) :reader pane-pixmaps)))
 > +  ((pixmaps :initform (make-hash-table :test #'eq) :reader pane-pixmaps)
 > +   (darker-gray-progressions :initform (make-array 10 :initial-element nil :adjustable t)
 > +			     :reader darker-gray-progressions)
 > +   (lighter-gray-progressions :initform (make-array 10 :initial-element nil :adjustable t)
 > +			      :reader lighter-gray-progressions)))

So far so good. 

 >  
 >  (defmethod dispatch-event :before ((pane score-pane) (event pointer-enter-event))
 >    (let ((port (port pane)))
 > @@ -474,10 +478,8 @@
 >      (when (stream-drawing-p *pane*)
 >        (medium-draw-rectangle* medium x1 y1 x2 y2 t))))
 >  
 > -(defparameter *darker-gray-progressions*
 > -  (make-array 10 :initial-element nil :adjustable t))
 > -(defparameter *lighter-gray-progressions*
 > -  (make-array 10 :initial-element nil :adjustable t))
 > +(defvar *darker-gray-progressions*)
 > +(defvar *lighter-gray-progressions*)

OK.

 >  
 >  ;;; don't delete this yet, since I don't know how the other one will work out. 
 >  ;; (defun ensure-gray-progressions (index)
 > @@ -563,7 +565,7 @@
 >  (defclass upward-beam-output-record (beam-output-record)
 >    ())
 >  
 > -(defmethod replay-output-record ((record upward-beam-output-record) stream
 > +(defmethod replay-output-record ((record upward-beam-output-record) (stream score-pane)

Should be fine. 

 >  				 &optional (region +everywhere+)
 >  				 (x-offset 0) (y-offset 0))
 >    (declare (ignore x-offset y-offset region))
 > @@ -572,15 +574,17 @@
 >        (let ((medium (sheet-medium stream)))
 >  	(let ((*light-glyph* (not (eq ink +black+))))
 >  	  (with-drawing-options (medium :ink ink)
 > -	    ;; we replay with the identity tranformation, so
 > -	    ;; we have to draw the other way
 > -	    (draw-downward-beam medium x1 (1- y2) (1- (+ y1 thickness)) thickness
 > -				(/ (- x2 x1) (- y2 y1 thickness)))))))))
 > +	    (let ((*lighter-gray-progressions* (lighter-gray-progressions stream))
 > +		  (*darker-gray-progressions* (darker-gray-progressions stream)))
 > +	      ;; we replay with the identity tranformation, so

While you are at it, you can fix the typo in "transformation". 

 > +	      ;; we have to draw the other way
 > +	      (draw-downward-beam medium x1 (1- y2) (1- (+ y1 thickness)) thickness
 > +				  (/ (- x2 x1) (- y2 y1 thickness))))))))))
 >  
 >  (defclass downward-beam-output-record (beam-output-record)
 >    ())
 >  
 > -(defmethod replay-output-record ((record downward-beam-output-record) stream
 > +(defmethod replay-output-record ((record downward-beam-output-record) (stream score-pane)
 >  				 &optional (region +everywhere+)
 >  				 (x-offset 0) (y-offset 0))
 >    (declare (ignore x-offset y-offset region))
 > @@ -589,10 +593,12 @@
 >        (let ((medium (sheet-medium stream)))
 >  	(let ((*light-glyph* (not (eq ink +black+))))
 >  	  (with-drawing-options (medium :ink ink)
 > -	    ;; we replay with the identity tranformation, so
 > -	    ;; we have to draw the other way
 > -	    (draw-upward-beam medium x1 (1- (+ y1 thickness)) (1- y2) thickness
 > -			      (/ (- x2 x1) (- y2 y1 thickness)))))))))
 > +	    (let ((*lighter-gray-progressions* (lighter-gray-progressions stream))
 > +		  (*darker-gray-progressions* (darker-gray-progressions stream)))
 > +	      ;; we replay with the identity tranformation, so

idem.

 > +	      ;; we have to draw the other way
 > +	      (draw-upward-beam medium x1 (1- (+ y1 thickness)) (1- y2) thickness
 > +				(/ (- x2 x1) (- y2 y1 thickness))))))))))
 >  
 >  (defun draw-sloped-beam (medium x1 y1 x2 y2 thickness inverse-slope)
 >    (let ((transformation (medium-transformation *pane*)))
 > @@ -658,6 +664,8 @@
 >    (let ((pixmap (gensym))
 >  	(mirror (gensym)))
 >      `(let* ((*pane* ,pane)
 > +	    (*lighter-gray-progressions* (lighter-gray-progressions pane))
 > +	    (*darker-gray-progressions* (darker-gray-progressions pane))
 >  	    (,pixmap (allocate-pixmap *pane* 800 900))
 >  	    (,mirror (sheet-direct-mirror *pane*)))
 >        (draw-rectangle* ,pixmap 0 0 800 900 :filled t :ink +white+)
 > 
 > Comments?

Looks fine to me.  I do not see why you find it uglier.  Good work. 

-- 
Robert Strandh

---------------------------------------------------------------------
Greenspun's Tenth Rule of Programming: any sufficiently complicated C
or Fortran program contains an ad hoc informally-specified bug-ridden
slow implementation of half of Common Lisp.
---------------------------------------------------------------------