310845 - [BeOS]Cleanup StringWidth methods in nsRenderingContextBeOS

Status: RESOLVED FIXED

(Core Graveyard :: GFX: BeOS, defect)

Description

[Sergei Dolgov]

Follow-up to
https://bugzilla.mozilla.org/show_bug.cgi?id=310708#c15
and
https://bugzilla.mozilla.org/show_bug.cgi?id=297074

Comment 1

[Sergei Dolgov]

Attachment: patch

simple and obvious patch. Uses regular GetWidh() method everywhere around
nsRenderingContext.
review request

Comment 2

[Sergei Dolgov]

Attachment: additional patch

Depends on first, and then enables even more fast string width measurment -
allowing summarize cached glyph widths to whole string width.
If first patch gets reviewed 
review request

Comment 3

[tqh]

Comment on attachment 198347 [details] [diff] [review]
patch

r=[e-mail]

Comment 4

[tqh]

Comment on attachment 198386 [details] [diff] [review]
additional patch

r=[e-mail]

The charlen = line looks a bit magic, with magic numbers. Maybe it should be
documented.

Comment 5

[Sergei Dolgov]

Landed in trunk:
nsFontMetricsBeOS.cpp
new revision: 1.44; previous revision: 1.43
nsRenderingContextBeOS.cpp
new revision: 1.56; previous revision: 1.55 

P.S. About charlen magic. Even in Be Newsletter where i found this, they wrote
something alike as comment: "Ohh, don't ask us how - but it works!":)

Comment 6

[Sergei Dolgov]

as for bigger fix - this is for future, when Bug 297074 will be completed and
fix landed. I can easily implement method for getting glyphs width array, but it
has no sense at moment.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

+		charlen = ((0xE5000000 >> ((*utf8str >> 3) & 0x1E)) & 3) + 1;

how about putting that into a "PRUint8 utf8_char_len(const char* uf8str)" function?

Comment 8

[Sergei Dolgov]

Heh, biesi, there is such function 15 lines or so above. Even inlined.
But as at moment we use this trick in only place, i decided to avoid this time
such little overhead and put code directly in GetStringWidth

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

inline functions cause no overhead, and I think using that function would make
this more readable... but ok, it's your module.

Comment 10

[Niels Reedijk]

Comment on attachment 198347 [details] [diff] [review]
patch

Requesting permission to land on the BUGZILLA_1_8_BRANCH.

This is a BeOS-only change and does not affect other platforms.

Note: sergei, apply this one after bug 310708.

Comment 11

[Mike Schroepfer]

Comment on attachment 198347 [details] [diff] [review]
patch

a=schrep for drivers for BeOS only change.

Comment 12

[Mike Schroepfer]

Comment on attachment 198386 [details] [diff] [review]
additional patch

a=schrep for drivers for BeOS only change.

Comment 13

[Sergei Dolgov]

applied two patches at once:
Checking in mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp;
/cvsroot/mozilla/gfx/src/beos/nsRenderingContextBeOS.cpp,v  <--  nsRenderingContextBeOS.cpp
new revision: 1.51.12.5; previous revision: 1.51.12.4
done