Re: Integer & glyph (trunk and emacs_unicode)

Re: Integer & glyph (trunk and emacs_unicode)

[Stefan Monnier]

> (defun make-glyph-code (char &optional face)
>   "Return a glyph code representing char CHAR with face FACE."
>   ;; Due to limitations on Emacs integer values, faces with
>   ;; face id greater that 4091 are silently ignored.
>   (if (and face (<= (face-id face) #xfff))
>       (logior char (lsh (face-id face) 19))
>     char))

> So, it assumes 12 bits for face id and 19 bits for char code,
> the result is an integer of 31 bits.

I suspect that the "19" used to be something else (e.g. 16 when
integers were 28bits, so 16+12=28).

In any case, the test should be made more robust by not assuming
anything about the available range of integers (which has changed over
the years and can change depending on your config).
How 'bout

   (let ((id (and face (face-id face)))
     (if (and (numberp id)
              ;; Due to limitations on Emacs integer values, only
              ;; face ids below a certain limit can be used.
              (= id (lsh (lsh id 19) -19)))
         (logior char (lsh id 19))
       char))


-- Stefan

Integer & glyph (trunk and emacs_unicode)

[Vinicius Jose Latorre]

It seems that glyphs have some problems as it's implemented
by make-glyph-code, glyph-char and glyph-face in disp-table.el file.

The integers in Emacs Lisp have a minimum range from -2**28 to
2**28 - 1, that is, 29 bits.  In my system (AMD Athlon 64 -
GNU/Linux Debian using Emacs 23.0.50.1 - CVS trunk) integers
in Emacs Lisp have 29 bits.

The make-glyph-code in Emacs 23.0.50.1 (CVS trunk) is:

 (defun make-glyph-code (char &optional face)
   "Return a glyph code representing char CHAR with face FACE."
   ;; Due to limitations on Emacs integer values, faces with
   ;; face id greater that 4091 are silently ignored.
   (if (and face (<= (face-id face) #xfff))
       (logior char (lsh (face-id face) 19))
     char))

So, it assumes 12 bits for face id and 19 bits for char code,
the result is an integer of 31 bits.

Shouldn't it be used #x3ff (10 bits for face id) instead of #xfff?

BTW, the same thing happens in Emacs unicode branch.

RE: Integer & glyph (trunk and emacs_unicode)

[Drew Adams]

> > (defun make-glyph-code (char &optional face)
> >   "Return a glyph code representing char CHAR with face FACE."
> >   ;; Due to limitations on Emacs integer values, faces with
> >   ;; face id greater that 4091 are silently ignored.
> >   (if (and face (<= (face-id face) #xfff))
> >       (logior char (lsh (face-id face) 19))
> >     char))
>
> > So, it assumes 12 bits for face id and 19 bits for char code,
> > the result is an integer of 31 bits.
>
> I suspect that the "19" used to be something else (e.g. 16 when
> integers were 28bits, so 16+12=28).
>
> In any case, the test should be made more robust by not assuming
> anything about the available range of integers (which has changed over
> the years and can change depending on your config).
> How 'bout
>
>    (let ((id (and face (face-id face)))
>      (if (and (numberp id)
>               ;; Due to limitations on Emacs integer values, only
>               ;; face ids below a certain limit can be used.
>               (= id (lsh (lsh id 19) -19)))
>          (logior char (lsh id 19))
>        char))

I'm not really following this, but is this about the change from `19' to
`22' in this code (this is my own version, to accommodate both the old and
new)?

(defun make-glyph-code (char &optional face)
  "Return a glyph code representing char CHAR with face FACE."
  (if face
      (logior char (lsh (face-id face)
                        (if (<= emacs-major-version 22) 19 22)))
    char))

That is, is this related to thread "bug of display-table & make-glyph-code,
2007-09-10?

Re: Integer & glyph (trunk and emacs_unicode)

[Vinicius Jose Latorre]

Drew Adams wrote:
>>> (defun make-glyph-code (char &optional face)
>>>   "Return a glyph code representing char CHAR with face FACE."
>>>   ;; Due to limitations on Emacs integer values, faces with
>>>   ;; face id greater that 4091 are silently ignored.
>>>   (if (and face (<= (face-id face) #xfff))
>>>       (logior char (lsh (face-id face) 19))
>>>     char))
>>>       
>>> So, it assumes 12 bits for face id and 19 bits for char code,
>>> the result is an integer of 31 bits.
>>>       
>> I suspect that the "19" used to be something else (e.g. 16 when
>> integers were 28bits, so 16+12=28).
>>
>> In any case, the test should be made more robust by not assuming
>> anything about the available range of integers (which has changed over
>> the years and can change depending on your config).
>> How 'bout
>>
>>    (let ((id (and face (face-id face)))
>>      (if (and (numberp id)
>>               ;; Due to limitations on Emacs integer values, only
>>               ;; face ids below a certain limit can be used.
>>               (= id (lsh (lsh id 19) -19)))
>>          (logior char (lsh id 19))
>>        char))
>>     

Yes, this fix the problem.

Also it could be made a constant for the number of bits for char like:

(defconst glyph-char-bits
   (if (<= emacs-major-version 22) 19 22)
   "Specify the number of bits used by a char code in a glyph.")

And the code:

    (let ((id (and face (face-id face))))
      (if (and (numberp id)
               ;; Due to limitations on Emacs integer values, only
               ;; face ids below a certain limit can be used.
               (= id (lsh (lsh id glyph-char-bits) (- glyph-char-bits))))
          (logior char (lsh id glyph-char-bits))
        char))

Is there any problem if I implement this solution in trunk and Emacs 
unicode?

>
> I'm not really following this, but is this about the change from `19' to
> `22' in this code (this is my own version, to accommodate both the old and
> new)?
>
> (defun make-glyph-code (char &optional face)
>   "Return a glyph code representing char CHAR with face FACE."
>   (if face
>       (logior char (lsh (face-id face)
>                         (if (<= emacs-major-version 22) 19 22)))
>     char))
>
> That is, is this related to thread "bug of display-table & make-glyph-code,
> 2007-09-10?
>   

No, the problem is this test:

    (if (and face (<= (face-id face) #xfff))

Which assumes 12 bits for face (constant #xFFF), it should be 10 bits 
(#x3FF).  This test in Emacs unicode should be with #x3F (7 bits).

But the solution given by Stefan is better as it is only needed to 
specify the number of bits used in a char code.

Re: Integer & glyph (trunk and emacs_unicode)

[Richard Stallman]

I recall that we decided to reimplement these glyphs as cons cells.
I also recall that we did some of the work.  Where does this stand?

Re: Integer & glyph (trunk and emacs_unicode)

[Richard Stallman]

	(let ((id (and face (face-id face))))
	  (if (and (numberp id)
		   ;; Due to limitations on Emacs integer values, only
		   ;; face ids below a certain limit can be used.
		   (= id (lsh (lsh id glyph-char-bits) (- glyph-char-bits))))
	      (logior char (lsh id glyph-char-bits))
	    char))

    Is there any problem if I implement this solution in trunk and Emacs 
    unicode?

It's not _wrong_, but we've already decided to replace the
mechanism with the use of cons cells.

And I am pretty sure someone has already written the code for that.
Who was it?

Re: Integer & glyph (trunk and emacs_unicode)

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

> 	(let ((id (and face (face-id face))))
> 	  (if (and (numberp id)
> 		   ;; Due to limitations on Emacs integer values, only
> 		   ;; face ids below a certain limit can be used.
> 		   (= id (lsh (lsh id glyph-char-bits) (- glyph-char-bits))))
> 	      (logior char (lsh id glyph-char-bits))
> 	    char))
>
>     Is there any problem if I implement this solution in trunk and Emacs 
>     unicode?
>
> It's not _wrong_, but we've already decided to replace the
> mechanism with the use of cons cells.
>
> And I am pretty sure someone has already written the code for that.
> Who was it?

I've promised to write the necessary code as soon as the unicode
branch has been merged.

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

Re: Integer & glyph (trunk and emacs_unicode)

[Richard Stallman]

    I've promised to write the necessary code as soon as the unicode
    branch has been merged.

Thanks.  So there is no point changing details now.