Hi Roman,
OK, I have finally integrated this fix (with
a couple of minor changes after internal
code review):
http://hg.openjdk.java.net/jdk7/2d/jdk/rev/4af4867ed787
From the code review by Chris (should have CC-ed you in the
first place):
>> SMF.java
>> 49: Take out the "Unix" part.
>
> Fixed.
>
>> 50,51: I don't think that's what it does.
>
> Removed the comment. If this is added later,
> will add back in.
>
>> 56, 61, 72, 84: Unnecessary blank lines.
>
> Removed.
>
>> 66: [EMAIL PROTECTED] null}
>
> Fixed.
>
>> 75: I'd prefer an IllegalArgumentException with an explanatory message
>> (e.g. "factory must be non-null").
>
> Fixed.
Thanks,
Dmitri
Dmitri Trembovetski wrote:
Hi Roman,
Roman Kennke wrote:
I can tell that someone didn't build on windows
since the build fails there =) The fix should have deleted
src/windows/classes/sun/java2d/SurfaceManagerFactory.java
just like it did the solaris/ one.
Whoops, did I forget that? I was sure I deleted it locally, so somehow
it didn't make it into the patch. Sorry for that.
No problem, I should have spotted it during review anyway.
I'm just not used to reviewing by looking at the raw diff
output, we use that webrev tool for generating html for
code reviews, much easier on the eyes.
In general, if the fix touches shared code it is
very advisable to build on all platforms - at
least on 32-bit solaris/linux/windows.
The problem is, I don't have a Windows box (easily) available. I will
have to setup one at work, including all the build machinery for
OpenJDK.
I myself use vmware to compile on windows if needed- then you'd only need
a windows license (and the compilers - although someone did compile
the jdk with the free VS2008, I believe). Of course, if you'll be
touching
any hw-acceleration related stuff, you'll need a real box to test on,
so you may as well get the box.
Thanks,
Dmitri
