A bug in tetris

A bug in tetris

[Leo]

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

Hi there,

I just noticed that tetris has a weird look in GNU Emacs 23.0.0.4
(i686-pc-linux-gnu, GTK+ Version 2.10.14) of 2007-07-27. Please see the
screenshot:


[-- Attachment #2: tetris.png --]
[-- Type: image/png, Size: 5697 bytes --]

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]


HTH,
-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

         Gnus is one component of the Emacs operating system.

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: A bug in tetris

[Johan Bockgård]

Leo <sdl.web@gmail.com> writes:

> I just noticed that tetris has a weird look in GNU Emacs 23.0.0.4
> (i686-pc-linux-gnu, GTK+ Version 2.10.14) of 2007-07-27. Please see the
> screenshot:

Looks like you have set line spacing to 1.

-- 
Johan Bockgård

Re: A bug in tetris

[Mathias Dahl]

> Leo <sdl.web@gmail.com> writes:
>
> > I just noticed that tetris has a weird look in GNU Emacs 23.0.0.4
> > (i686-pc-linux-gnu, GTK+ Version 2.10.14) of 2007-07-27. Please see the
> > screenshot:
>
> Looks like you have set line spacing to 1.

Then maybe Tetris and other games should set it to 0 locally?

Re: A bug in tetris

[Johan Bockgård]

"Mathias Dahl" <mathias.dahl@gmail.com> writes:

>> Leo <sdl.web@gmail.com> writes:
>>
>> > I just noticed that tetris has a weird look in GNU Emacs 23.0.0.4
>> > (i686-pc-linux-gnu, GTK+ Version 2.10.14) of 2007-07-27. Please see the
>> > screenshot:
>>
>> Looks like you have set line spacing to 1.
>
> Then maybe Tetris and other games should set it to 0 locally?


2007-08-19  Johan Bockgård  <bojohan@dd.chalmers.se>

	* play/gamegrid.el (gamegrid-init): Set line-spacing to 0.



--- gamegrid.el	26 Jul 2007 18:02:08 +0200	1.29
+++ gamegrid.el	19 Aug 2007 17:32:53 +0200	
@@ -386,6 +386,7 @@
 (defun gamegrid-init (options)
   (setq buffer-read-only t
 	truncate-lines t
+	line-spacing 0
 	gamegrid-display-options options)
   (buffer-disable-undo (current-buffer))
   (gamegrid-initialize-display))


-- 
Johan Bockgård

Re: A bug in tetris

[Mathias Dahl]

On 8/19/07, Johan Bockgård <bojohan+news@dd.chalmers.se> wrote:

> > Then maybe Tetris and other games should set it to 0 locally?
>
>
> 2007-08-19  Johan Bockgård  <bojohan@dd.chalmers.se>
>
>         * play/gamegrid.el (gamegrid-init): Set line-spacing to 0.
>

You win the price for today's fastest patch! :)

Re: A bug in tetris

[Leo]

On 2007-08-19 16:07 +0100, Johan Bockgård wrote:
> Leo <sdl.web@gmail.com> writes:
>
>> I just noticed that tetris has a weird look in GNU Emacs 23.0.0.4
>> (i686-pc-linux-gnu, GTK+ Version 2.10.14) of 2007-07-27. Please see the
>> screenshot:
>
> Looks like you have set line spacing to 1.

line-spacing is nil in 'emacs -Q'.

-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

         Gnus is one component of the Emacs operating system.

Re: A bug in tetris

[Leo]

On 2007-08-19 16:37 +0100, Johan Bockgård wrote:
> "Mathias Dahl" <mathias.dahl@gmail.com> writes:
>
>>> Leo <sdl.web@gmail.com> writes:
>>>
>>> > I just noticed that tetris has a weird look in GNU Emacs 23.0.0.4
>>> > (i686-pc-linux-gnu, GTK+ Version 2.10.14) of 2007-07-27. Please see the
>>> > screenshot:
>>>
>>> Looks like you have set line spacing to 1.
>>
>> Then maybe Tetris and other games should set it to 0 locally?
>
>
> 2007-08-19  Johan Bockgård  <bojohan@dd.chalmers.se>
>
> 	* play/gamegrid.el (gamegrid-init): Set line-spacing to 0.
>
>
>
> --- gamegrid.el	26 Jul 2007 18:02:08 +0200	1.29
> +++ gamegrid.el	19 Aug 2007 17:32:53 +0200	
> @@ -386,6 +386,7 @@
>  (defun gamegrid-init (options)
>    (setq buffer-read-only t
>  	truncate-lines t
> +	line-spacing 0
>  	gamegrid-display-options options)
>    (buffer-disable-undo (current-buffer))
>    (gamegrid-initialize-display))

I am still seeing the same problem with this patch. The default value of
line-spacing is nil which means no extra space.

-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

         Gnus is one component of the Emacs operating system.

Re: A bug in tetris

[Sven Joachim]

Leo <sdl.web@gmail.com> writes:

> I am still seeing the same problem with this patch. The default value of
> line-spacing is nil which means no extra space.

You may try to use a smaller font.  It is possible that the height
Emacs has to allocate for the font is greater than the height of the
Tetris glyphs.  In Emacs 22, this happens if you use the courier font
and a font size > 12.  The font in your screenshot did not look too
big, but anyway...

Cheers,
       Sven

Re: A bug in tetris

[Leo]

On 2007-08-19 19:27 +0100, Sven Joachim wrote:
> Leo <sdl.web@gmail.com> writes:
>
>> I am still seeing the same problem with this patch. The default value of
>> line-spacing is nil which means no extra space.
>
> You may try to use a smaller font.  It is possible that the height
> Emacs has to allocate for the font is greater than the height of the
> Tetris glyphs.  In Emacs 22, this happens if you use the courier font
> and a font size > 12.  The font in your screenshot did not look too
> big, but anyway...
>
> Cheers,
>        Sven

I have tested font size from pixelsize from 12 to 24 and when pixelsize
is bigger than 14 (inclusive), I can see the stripes. However the gap
stays constant.

-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

         Gnus is one component of the Emacs operating system.

Re: A bug in tetris

[Kim F. Storm]

Leo <sdl.web@gmail.com> writes:

> I am still seeing the same problem with this patch. The default value of
> line-spacing is nil which means no extra space.


Maybe the newlines in the gamegrid must be marked to not increase
line height by its own:

   (propertize "\n" 'line-height t)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: A bug in tetris

[Richard Stallman]

That is a good idea.  Please install your patch.

Re: A bug in tetris

[Kenichi Handa]

In article <87hcmvp3bn.fsf@kfs-lx.testafd.dk>, storm@cua.dk (Kim F. Storm) writes:

> Leo <sdl.web@gmail.com> writes:
> > I am still seeing the same problem with this patch. The default value of
> > line-spacing is nil which means no extra space.


> Maybe the newlines in the gamegrid must be marked to not increase
> line height by its own:

>    (propertize "\n" 'line-height t)

Isn't it better to make that the default behaviour if a line
is not empty?

---
Kenichi Handa
handa@m17n.org

Re: A bug in tetris

[Kim F. Storm]

Kenichi Handa <handa@m17n.org> writes:

> In article <87hcmvp3bn.fsf@kfs-lx.testafd.dk>, storm@cua.dk (Kim F. Storm) writes:
>
>> Leo <sdl.web@gmail.com> writes:
>> > I am still seeing the same problem with this patch. The default value of
>> > line-spacing is nil which means no extra space.
>
>
>> Maybe the newlines in the gamegrid must be marked to not increase
>> line height by its own:
>
>>    (propertize "\n" 'line-height t)
>
> Isn't it better to make that the default behaviour if a line
> is not empty?

It would have been an incompatible change, so I didn't do that.

However, I think there might be good reasons to do just that,
and reverse the meaning of the line-height == t to mean _do_
include the newline of a non-empty line in the height calculation.

If we want to do that, I think _NOW_ is the only time to do it.  The
feature is new in 22.1, so probably nobody has used it outside the
code in emacs itself, so a change should be made before releasing 22.2.

(remember to change the uses already in the code).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: A bug in tetris

[Richard Stallman]

    >  	truncate-lines t
    > +	line-spacing 0
    >  	gamegrid-display-options options)
    >    (buffer-disable-undo (current-buffer))
    >    (gamegrid-initialize-display))

    I am still seeing the same problem with this patch. The default value of
    line-spacing is nil which means no extra space.

I wonder if there is something strange about the default font on nyour
machine.

Handa-san, can you help, by telling him how to investigate?

Re: A bug in tetris

[Leo]

On 2007-08-20 16:16 +0100, Richard Stallman wrote:
>     >  	truncate-lines t
>     > +	line-spacing 0
>     >  	gamegrid-display-options options)
>     >    (buffer-disable-undo (current-buffer))
>     >    (gamegrid-initialize-display))
>
>     I am still seeing the same problem with this patch. The default value of
>     line-spacing is nil which means no extra space.
>
> I wonder if there is something strange about the default font on nyour
> machine.
>
> Handa-san, can you help, by telling him how to investigate?

The font I am using is:
,----
|         character: T (84, #o124, #x54)
| preferred charset: ascii (ASCII (ISO646 IRV))
|        code point: 0x54
|            syntax: w 	which means: word
|          category: a:ASCII graphic characters 32-126 (ISO646 IRV:1983[4/0]) l:Latin r:Japanese roman
|       buffer code: #x54
|         file code: not encodable by coding system utf-8-emacs
|           display: by this font (glyph code)
|      dejavu lgc sans mono:pixelsize=16:foundry=unknown:weight=medium:slant=r:width=normal (#x37)
| 
| Character code properties are not shown: customize what to show
| 
| There are text properties here:
|   auto-composed        t
|   fontified            t
`----

-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

         Gnus is one component of the Emacs operating system.

Re: A bug in tetris

[Richard Stallman]

    > Maybe the newlines in the gamegrid must be marked to not increase
    > line height by its own:

    >    (propertize "\n" 'line-height t)

    Isn't it better to make that the default behaviour if a line
    is not empty?

Did we discuss this question when that feature was added?
Was there any particular reason to handle this the way we did?

Re: A bug in tetris

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     > Maybe the newlines in the gamegrid must be marked to not increase
>     > line height by its own:
>
>     >    (propertize "\n" 'line-height t)
>
>     Isn't it better to make that the default behaviour if a line
>     is not empty?
>
> Did we discuss this question when that feature was added?

I don't remember.

> Was there any particular reason to handle this the way we did?

I answered (vaguely) yesterday, but I now recall the reasoning
for not making it standard:

In most cases, I would assume that if you put a face
(with a smaller line height) on the text on a whole line, 
you also put that face on the newline if you want the whole
line to have the smaller line height.

If you leave the default face on the newline, you (probably)
want to have the default line spacing for the line even if the
text on the line is in a smaller face.

So this is the current default behaviour (as it was in 21.x)
-- which IMO is correct for normal use cases.

In the rare case where a line is in the default face, but only
contains (small height) images (e.g. in a game grid or a sliced image)
do you get into a case where the height of the newline may get into
the way -- that is why I made a special line-height=t property for the
newline to handle such unusual cases.

The insert-sliced-image function uses this property.

Game grid functions should do that too.

Conclusion: I think we should keep the current default.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: A bug in tetris

[Kenichi Handa]

In article <87sl6dp9tg.fsf@kfs-lx.testafd.dk>, storm@cua.dk (Kim F. Storm) writes:

> I answered (vaguely) yesterday, but I now recall the reasoning
> for not making it standard:

> In most cases, I would assume that if you put a face
> (with a smaller line height) on the text on a whole line, 
> you also put that face on the newline if you want the whole
> line to have the smaller line height.

> If you leave the default face on the newline, you (probably)
> want to have the default line spacing for the line even if the
> text on the line is in a smaller face.

> So this is the current default behaviour (as it was in 21.x)
> -- which IMO is correct for normal use cases.

> In the rare case where a line is in the default face, but only
> contains (small height) images (e.g. in a game grid or a sliced image)
> do you get into a case where the height of the newline may get into
> the way -- that is why I made a special line-height=t property for the
> newline to handle such unusual cases.

> The insert-sliced-image function uses this property.

> Game grid functions should do that too.

> Conclusion: I think we should keep the current default.

I don't have a strong objection to it.  Anyway, I found that
the current problem is in the different place.  gamegrid.el
has this function.

(defun gamegrid-setup-default-font ()
  (setq gamegrid-face
	(copy-face 'default
		   (intern (concat "gamegrid-face-" (buffer-name)))))
  (when (eq gamegrid-display-mode 'glyph)
    (let ((max-height nil))
      (loop for c from 0 to 255 do
	    (let ((glyph (aref gamegrid-display-table c)))
	      (when (and (listp glyph) (eq (car  glyph) 'image))
		(let ((height (cdr (image-size glyph))))
		  (if (or (null max-height)
			  (< max-height height))
		      (setq max-height height))))))
      (when (and max-height (< max-height 1))
	(set-face-attribute gamegrid-face nil :height max-height)))))

It tries to make a face height shorter than the grid-glyph
height.  I confirmed that when I change the last line to:

	(set-face-attribute gamegrid-face nil :height (- max-height 0.1))))))

the resulting face is good and doesn't produce 1-dot
horizontal gap in the play field.   I'm now investigating
why the original code isn't good enough.

---
Kenichi Handa
handa@m17n.org

Re: A bug in tetris

[Richard Stallman]

    Conclusion: I think we should keep the current default.

Your argument seems valid to me.

Re: A bug in tetris

[Kenichi Handa]

In article <E1INSWq-0003IU-BH@etlken.m17n.org>, Kenichi Handa <handa@m17n.org> writes:

> I don't have a strong objection to it.  Anyway, I found that
> the current problem is in the different place.  gamegrid.el
> has this function.

> (defun gamegrid-setup-default-font ()
>   (setq gamegrid-face
> 	(copy-face 'default
> 		   (intern (concat "gamegrid-face-" (buffer-name)))))
>   (when (eq gamegrid-display-mode 'glyph)
>     (let ((max-height nil))
>       (loop for c from 0 to 255 do
> 	    (let ((glyph (aref gamegrid-display-table c)))
> 	      (when (and (listp glyph) (eq (car  glyph) 'image))
> 		(let ((height (cdr (image-size glyph))))
> 		  (if (or (null max-height)
> 			  (< max-height height))
> 		      (setq max-height height))))))
>       (when (and max-height (< max-height 1))
> 	(set-face-attribute gamegrid-face nil :height max-height)))))

> It tries to make a face height shorter than the grid-glyph
> height.  I confirmed that when I change the last line to:

> 	(set-face-attribute gamegrid-face nil :height (- max-height 0.1))))))

> the resulting face is good and doesn't produce 1-dot
> horizontal gap in the play field.   I'm now investigating
> why the original code isn't good enough.

The source of the problem is the rounding off done while
converting a point size to pixel size.  In my environment,
resolution of the screen is 96dpi, grid-glyph height is
16dots, canonical char height is 20dots, the default font
height is 12.1pt, thus the requested height of the font is
9.6pt (== 12.1 * (16 / 20)).  The corresponding pixel size
is 12.75 (== 96 * (9.6 / 72.27)).  So, it's rounded off to 13,
but what we want here is a font of 12 pixel size.

So, I've just installed the attached change.  It may be
possible to add a special face attribute to tell not to
choose a font that is larger than the requested size, but
that require many C code changes.  In addition, I think such
a case (need a font equal to or smaller than a requested
size) is rare.

---
Kenichi Handa
handa@m17n.org

Index: gamegrid.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/play/gamegrid.el,v
retrieving revision 1.16.4.12
retrieving revision 1.16.4.13
diff -u -r1.16.4.12 -r1.16.4.13
--- gamegrid.el	27 Jul 2007 10:47:51 -0000	1.16.4.12
+++ gamegrid.el	23 Aug 2007 12:13:24 -0000	1.16.4.13
@@ -320,7 +320,14 @@
 			  (< max-height height))
 		      (setq max-height height))))))
       (when (and max-height (< max-height 1))
-	(set-face-attribute gamegrid-face nil :height max-height)))))
+	(let ((default-font-height (face-attribute 'default :height))
+	      (resy (/ (display-pixel-height) (/ (display-mm-height) 25.4)))
+	      point-size pixel-size)
+	  (setq point-size (/ (* (float default-font-height) max-height) 10)
+		pixel-size (floor (* resy (/ point-size 72.27)))
+		point-size (* (/ pixel-size resy) 72.27))
+	  (set-face-attribute gamegrid-face nil
+			      :height (floor (* point-size 10))))))))
 
 (defun gamegrid-initialize-display ()
   (setq gamegrid-display-mode (gamegrid-display-type))

Re: A bug in tetris

[Leo]

On 2007-08-23 13:17 +0100, Kenichi Handa wrote:
> In article <E1INSWq-0003IU-BH@etlken.m17n.org>, Kenichi Handa <handa@m17n.org> writes:
>
>> I don't have a strong objection to it.  Anyway, I found that
>> the current problem is in the different place.  gamegrid.el
>> has this function.
>
>> (defun gamegrid-setup-default-font ()
>>   (setq gamegrid-face
>> 	(copy-face 'default
>> 		   (intern (concat "gamegrid-face-" (buffer-name)))))
>>   (when (eq gamegrid-display-mode 'glyph)
>>     (let ((max-height nil))
>>       (loop for c from 0 to 255 do
>> 	    (let ((glyph (aref gamegrid-display-table c)))
>> 	      (when (and (listp glyph) (eq (car  glyph) 'image))
>> 		(let ((height (cdr (image-size glyph))))
>> 		  (if (or (null max-height)
>> 			  (< max-height height))
>> 		      (setq max-height height))))))
>>       (when (and max-height (< max-height 1))
>> 	(set-face-attribute gamegrid-face nil :height max-height)))))
>
>> It tries to make a face height shorter than the grid-glyph
>> height.  I confirmed that when I change the last line to:
>
>> 	(set-face-attribute gamegrid-face nil :height (- max-height 0.1))))))
>
>> the resulting face is good and doesn't produce 1-dot
>> horizontal gap in the play field.   I'm now investigating
>> why the original code isn't good enough.
>
> The source of the problem is the rounding off done while
> converting a point size to pixel size.  In my environment,
> resolution of the screen is 96dpi, grid-glyph height is
> 16dots, canonical char height is 20dots, the default font
> height is 12.1pt, thus the requested height of the font is
> 9.6pt (== 12.1 * (16 / 20)).  The corresponding pixel size
> is 12.75 (== 96 * (9.6 / 72.27)).  So, it's rounded off to 13,
> but what we want here is a font of 12 pixel size.
>
> So, I've just installed the attached change.  It may be
> possible to add a special face attribute to tell not to
> choose a font that is larger than the requested size, but
> that require many C code changes.  In addition, I think such
> a case (need a font equal to or smaller than a requested
> size) is rare.
[...]

Thanks. The patch works great.

-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

         Gnus is one component of the Emacs operating system.