From: Thomas Martitz <thomas.martitz_at_student.htw-berlin.de>
Date: Sat, 30 Oct 2010 16:27:58 +0200

Am 30.10.2010 15:36, schrieb Jonathan Gordon:
> ======== QUOTE ==========
>
> Date: 2010-10-30 01:12:08 +0200 (Sat, 30 Oct 2010)
> New Revision: 28386
>
> Log Message:
> Initialize (instantiate) RockboxFramebuffer from the C code like with
> the othe java objects.
> Remove some _at_Override annotations to make the Java code build with
> certain javac versions.
>
> Modified:
> trunk/android/src/org/rockbox/RockboxActivity.java
> trunk/android/src/org/rockbox/RockboxFramebuffer.java
> trunk/android/src/org/rockbox/RockboxPCM.java
> trunk/android/src/org/rockbox/RockboxService.java
> trunk/firmware/target/hosted/android/lcd-android.c
>
> ======== END QUOTE ==========
>
> Why was this done? I think it was alot cleaner with the java
> initialising this class. JNI isnt really nice and we should try
> limiting the amount of calls from C->java, especially classes like the
> framebuffer which are logically linked to the java side more than c
> (which should only need to have access to that classes lcd_update()
> and lcd_update_rect(), all other functions go java -> c ).
>

It was basically the same way before, C called a java_lcd_init() to pass
the lcd dimensions and the framebuffer pointer to Java. Now it does
little more, instantiation of the object (that's what the PCM and
Timer/Kernel driver also do). That's only a slight difference.

" JNI isnt really nice" is your personal opinion which I do not agree with.

The native C part is our authority, not the Java part. And the Java is
only our glue to the hardware, our HAL. So all initialization should be
coming from the C. That's how it's done throughout the target tree.

We should prefer C->Java calls over Java->C calls instead of limiting them.

And actually this commit makes it easier to restart the Service (see below).


> Furthermore, the way fb is being used is I think rather nasty. We are
> checking its resistance to see if rockbox is actually running instead
> of Doing It Properly.
>

Yes it's a bit tricky, but that has nothing to do with the commit. If we
can come up with something cleaner then great.

> Doing it this way also possibly limits our options with handling the
> different screen sizes (and multiple screens - i.e widgets - ) in a
> single build. Java should be telling C "use 480x800" not the other way
> around. (Obviously once LCD_WIDTH/HEIGHT are changed to runtime
> variables).
>

But it's the C part that's tied to the resolution, not the Java. There's
a patch for fixing this, then Java could tell C the resolution. But
until then C needs to tell Java what resolution it was compiled for.


> Another reason this is bad is how do we clean up resources to do a
> clean restart of the service? before this we could have (in theory)
> just killed the thread and not wasted any java objects, now we rely on
> the vm to clean up alot more (I don't know if that is actually a
> reasonable expectation here or not).
>
>

We'd simply return from main() (e.g. via setjmp/longjmp or just exit()).
Then the VM automatically frees all objects instantiated from the C
part, since they're by default local references (all objects from
jni->NewObject() are local references). Our drivers only create local
referenced objects. So returning from main and re-entering does the
needed cleanup and re-initialization. We don't need any cleanup. And
that's a very reasonable expectation (it's guaranteed).


Why don't you write a separate email for issues that have nothing to do
with the commit you're referring to?

Best regards.
Received on 2010-10-30