On Wed, May 1, 2013 at 9:30 PM, Phil Race <philip.r...@oracle.com> wrote: > Volker .. thanks for the patch looks good although I ask > that you break the source code lines at no more than 80 chars .. > that's the norm/standard we have always used >
I read the guidlines but after I realized that some of the files I changed already contain lines with more than 130 characters I thought that the guildines may be antiquated in that respect :) Nevertheless, I'm not against coding standards at all, so please find the '80 chars'-version of my patch below: http://cr.openjdk.java.net/~simonis/webrevs/7191872 > There actually is an open bug on this you can use :- > > 7191872: The panel does not include any information for each case on > solaris11-sparc. > Thanks. I've used it in the new webrev. Unfortunately searching the bug database for 'xrender' only returns a single hit and omits the bug you mentioned. But that's a totally different discussion :) Regards, Volker > > -phil. > > > On 4/26/2013 7:02 AM, Volker Simonis wrote: >> >> Hi, >> >> I've found and fixed a problem of the XRender pipeline on 64-bit >> Big-endian architectures which leads to the effect that all AWT/Swing >> applications will run as expected but won't actually display any >> character glyphs at all! This seems to be a "day-one" bug which still >> exists in both, JDK7 and JDK8 but probably didn't pop up until now >> because the XRender backend wasn't enabled by default. This has >> however changed recently in JDK8. >> >> Could somebody please open a bug for this issue and review the change: >> >> http://cr.openjdk.java.net/~simonis/webrevs/webrev_xrender.v1/ >> >> The fix is small and straightforward. >> You can read the full story below (or the HTML-version in the webrev >> for a better reading experience). >> >> Thank you and best regards, >> Volker >> >> >> >> Problem description >> >> The XRender backend writes some native data-structures which have the >> size of a native pointer (i.e. 32-bit on 32-bit architectures and >> 64-bit on 64-bit architectures) by using Unsafe.putInt(). This is of >> course not safe and breaks on 64-bit Big-endian architectures like >> SPARC, PPC64 or HPUX/IA64. The concrete problem is in the method >> setGlyphID in src/solaris/classes/sun/font/XRGlyphCacheEntry.java: >> >> public static void setGlyphID(long glyphInfoPtr, int id) { >> StrikeCache.unsafe.putInt(glyphInfoPtr + StrikeCache.cacheCellOffset, >> id); >> } >> >> which writes the offending field and in the function >> Java_sun_java2d_xr_XRBackendNative_XRAddGlyphsNative in >> src/solaris/native/sun/java2d/x11/XRBackendNative.c which reads this >> field: >> >> for (i=0; i < glyphCnt; i++) { >> GlyphInfo *jginfo = (GlyphInfo *) jlong_to_ptr(glyphInfoPtrs[i]); >> >> gid[i] = (Glyph) (0x0ffffffffL & ((unsigned long)(jginfo->cellInfo))); >> >> Actually, they both access the cellInfo field of a GlypInfo structure >> which has the size of a native pointer (see >> src/share/native/sun/font/fontscalerdefs.h): >> >> struct GlyphInfo { >> float advanceX; >> float advanceY; >> UInt16 width; >> UInt16 height; >> UInt16 rowBytes; >> UInt8 managed; >> float topLeftX; >> float topLeftY; >> void *cellInfo; >> UInt8 *image; >> } >> >> The native reader reads the field as a void* and casts it into an >> unsigned long. On Big-endian architectures this shifts the integer >> value which had been previously written in setGlyphID() by 32 bits to >> the left such that the final 'anding' with 0x0ffffffffL will always >> lead to a zero result (on Little-endian architectures there's no >> problem because the 4-byte integer value lies in the 'right' half of >> the 8-byte long value). >> >> How to reproduce the problem >> >> This finally leads to the effect that all AWT/Swing applications will >> run as expected but won't actually display any character glyphs at >> all! This behavior is reproducible with every JDK7 and JDK8. You just >> have to set the property -Dsun.java2d.xrender=True and make sure that >> the actual Xserver which displays the application window really >> supports the XRender extension. Using -Dsun.java2d.xrender=True (with >> capital 'T') will print a debugging line which confirms whether the >> XRender pipeline will be used ('XRender pipeline enabled') or not >> ('Could not enable XRender pipeline'). >> >> Notice that the XRender pipeline was switched off by default in JDK7 >> but is currently switched on by default in JDK8. However if the >> displaying X-server doesn't support XRender there's always a silent >> fallback to the plain X11 rendering pipeline. So you need an XRender >> capable X server to reproduce this problem. >> >> How to fix the problem >> >> Fixing the problem is easy: we just have to replace the offending >> Unsafe.putInt()/Unsafe.getInt() accesses with corresponding >> Unsafe.putAddress()/Unsafe.getAddress() calls. These routines >> write/read 4 byte in 32-bit VMs and 8 byte on 64-bit VMs (actually >> they read/write Unsafe.addressSize() bytes which corresponds to the >> mentioned values). >> >> Other cleanups in the patch >> >> Once I've fixed the problem, I've also cleaned up several other >> occurrences of the following pattern: >> >> long pixelDataAddress; >> if (StrikeCache.nativeAddressSize == 4) { >> pixelDataAddress = 0xffffffff & >> StrikeCache.unsafe.getInt(glyphInfoPtr + StrikeCache.pixelDataOffset); >> } else { >> pixelDataAddress = StrikeCache.unsafe.getLong(glyphInfoPtr + >> StrikeCache.pixelDataOffset); >> } >> long pixelDataAddress = StrikeCache.unsafe.getAddress(glyphInfoPtr + >> StrikeCache.pixelDataOffset); >> >> The intention of this code is to load a native pointer and store it >> into a Java long variable. On 32-bit architectures it uses >> Unsafe.getInt() to read the value and masks the result with 0xffffffff >> after converting it into a Java long value (which is actually the same >> as doing a zero-extension of a 32-bit value during the conversion into >> a 64-bit value). On 64-bit architectures it simply uses >> Unsafe.getLong() to read the value. >> >> The previous code can be simplified into a single expression by always >> using Unsafe.getAddress(): >> >> long pixelDataAddress = StrikeCache.unsafe.getAddress(glyphInfoPtr + >> StrikeCache.pixelDataOffset); >> >> because Unsafe.getAddress() does exactly the same thing: it reads a >> native pointer and returns it as a Java long value. In the case where >> the native pointer is less than 64 bits wide, it is extended as an >> unsigned number to a Java long (see JavaDoc of >> sun.misc.Unsafe.getAddress()). >> >> I've also simplified the following line in the function >> Java_sun_java2d_xr_XRBackendNative_XRAddGlyphsNative in >> src/solaris/native/sun/java2d/x11/XRBackendNative.c: >> >> - gid[i] = (Glyph) (0x0ffffffffL & ((unsigned >> long)(jginfo->cellInfo))); >> + // 'jginfo->cellInfo' is of type 'void*' (see definition of >> 'GlyphInfo' in fontscalerdefs.h) >> + // 'Glyph' is typedefed to 'unsigned long' (see >> http://www.x.org/releases/X11R7.7/doc/libXrender/libXrender.txt) >> + // Maybe we should assert that (sizeof(void*) == sizeof(Glyph)) ? >> + gid[i] = (Glyph) (jginfo->cellInfo); >> >> because Glyph is typedefed to unsigned long anyway. The >> 'zero-extension' of the result is unnecessary as well because we're >> actually reading an integer value here and not a pointer. > >
