markfguerra / GLWallpaperService

Please submit issues and pull requests to the main repository
https://github.com/GLWallpaperService/GLWallpaperService
Apache License 2.0
218 stars 114 forks source link

Make it more generally useable #18

Closed timogriese closed 12 years ago

timogriese commented 12 years ago

Hi,

as I announced before I have a patch ready for you that might be useful for users.

The main aspect of this patch is to remove those redundant interfaces and use the android supplied ones. The reasoning about this is that a custom renderer class could be used for a live wallpaper and in an GL enabled activity likewise. This speeds up testing for me a lot.

Please let me know if you like the idea.

Best regards, Timo

markfguerra commented 12 years ago

Hi Timo,

First of all, thanks for sending this in and spending the time to work it out. I understand you correctly, you're testing using an Activity and GLSurfaceView instead of doing so in an actual live wallpaper. This is something I like to do also, the workflow is nicer for me.

The thing is, I'm uneasy about changing public interfaces unless I really must. The problem is that developers are already using them, so changing things around could break things in the field.

However like I said there's a real use case for what you're doing. In fact, a few months ago I had come up with an Adapter class that makes GLWallpaperService renderers usable in GLSurfaceView, and vice versa. Basically if you wrap your Renderer class in the adapter, regardless of what type it is, you could use in either place.

I forgot all about it, but after looking over your post and code I decided to resurrect the idea. I made that class and some example code available in a new branch called 'adapter'.

https://github.com/markfguerra/GLWallpaperService/tree/archive/adapter (Branch deleted. Linked to a tag)

Please have a look and let me know if the RendererAdapter class meets your needs.

Mark

timogriese commented 12 years ago

Hi Mark,

I have put some thought into your argument. It seems valid to me to not change the public interface. An alternative approach I would favor more is to use the adapter to provide backward compatibility with your custom interface for those who use it. The main classes I would change to the Android interfaces. This should be helping to push developers to use the Android interfaces and keeps the ability to use your custom interfaces.

For a second thought. Couldn't one just use something like:

CustomInterface extends AndroidInterface

The signature seems to be equal in most (all?) cases. Just a thought.

Best regards, Pro

markfguerra commented 12 years ago

Interesting idea. I'll play with extending the GLSurfaceView.Renderer interface and I'll let you know how that works out.

timogriese commented 12 years ago

Please keep me updated :)

2011/12/21 Mark F Guerra < reply@reply.github.com

Interesting idea. I'll play with extending the GLSurfaceView.Renderer interface and I'll let you know how that works out.


Reply to this email directly or view it on GitHub:

https://github.com/markfguerra/GLWallpaperService/pull/18#issuecomment-3233991

jgooderham commented 12 years ago

I use min3d for a few of my papers. Min3d's renderer is a typical GLSurfaceView.Renderer. I just use a small wrapper class like so:

package min3d.core;

public class Min3dRenderer extends min3d.core.Renderer implements net.rbgrn.android.glwallpaperservice.GLWallpaperService.Renderer {

public Min3dRenderer( Scene scene ) {

    super( scene );
}
}

With this I could still use any typical GLSurfaceView.Renderer in an activity, and just use a similar wrapper class for the wallpaper service. You're implementing GLWallpaperService.Renderer and all the GLSurfaceView.Renderer relevant methods just pass on through.

markfguerra commented 12 years ago

Cool. Just to clarify, you're extending the class min3d.core.Renderer which implements GLSurfaceView.Renderer. You're also declaring that you implement GLWallpaperService.Renderer. So in effect your Renderer class is implementing both GLSurfaceView.Renderer and GLWallpaperService.Renderer. This is similar to what @Projekt2501 was suggesting above, except you accomplish it in a different way.

jgooderham commented 12 years ago

Yea and this way you can accomplish the intended goal of being able to re-use your renderer without making any changes to GLWallpaperService.

I looked over @Projekt2501's code but I have to admit I've never really understood the EGL specific stuff very well. I would like to ask if the above changes would make it any easier to start using openGL ES 2 as a optional rendering path. I did once try to weed my way through creating a GL2 context but I didn't succeed. It was a while back but if I recall I did have to hack around GLWallpaperService a bit.

Have you managed to create a GL2 context with the existing GLWallpaperService code? If that's not possible, then would any of the above changes facilitate GL2 context creation? I think it would be a worthwhile goal to build this into GLWallpaperService as I would like to start using GL2 in future projects. Either way it would be nice to have some kind of example for how to use GLWallpaperService with GL2.

markfguerra commented 12 years ago

I'm not an EGL expert either. It sure would be nice to have one around for questions like yours.

I have never tried to create a GL2 context myself, because I never needed to. Correct me if I'm mistaken, but I do not believe that this code would help you create one.

jgooderham commented 12 years ago

Ok here's where I was poking around a while back:

http://www.rozengain.com/blog/2011/08/25/rajawali-tutorial-2-creating-a-live-wallpaper-and-importing-a-model/

He's also using Robert Green's GLWallpaperService, but not the version here that's merged into one java file. Now that I look at it, why exactly are all these interfaces redefined in GLWallpaperService? Why not just use the official ones?

markfguerra commented 12 years ago

To be honest, I'm not sure why the interfaces were redefined; I didn't make that choice.

timogriese commented 12 years ago

One reason I can think of is the flexibility to be able to "upgrade" the custom interface. Maybe the one that made the decision had further plans.

2011/12/21 Mark F Guerra < reply@reply.github.com

To be honest, I'm not sure why the interfaces were redefined; I didn't make that choice.


Reply to this email directly or view it on GitHub:

https://github.com/markfguerra/GLWallpaperService/pull/18#issuecomment-3239894

markfguerra commented 12 years ago

I implemented @Projekt2501's suggestion to have the interface GLWallpaperService.Renderer extend GLSurfaceView.Renderer. The results have been published to a new branch, renderer-interface https://github.com/markfguerra/GLWallpaperService/commits/renderer-interface

I tested it out and it worked well for me. I compiled a jar which I loaded into the GLWallpaperExample project. It works just fine in an Activity or Wallpaper. There were no compilation issues and no hoops to jump through.

A JAR with the changes is available here: http://files.markguerra.net/glwallpaperservice/pull18/GLWallpaperService.jar-1146b2a.zip

This is actually a much better solution than the RendererAdapter class I wrote. There's less overhead involved in terms of execution time, maintenance, and usability for end users.

If anyone interested wishes to test this, please let us know how it works out for you.

amadaden commented 12 years ago

I have not had a chance to extensively test this yet but from what I see the changes look good and simple. The only way I can think of that these changes could cause a problem is if someone did something really weird like if ( someObject instanceof GLSurfaceView.Renderer) That if block would now fire if someObject was a GLWallpaperService.Renderer where it would not before.

timogriese commented 12 years ago

Hi,

I admit you have a point. From my experience there is mostly exactly one Renderer object in the app and thus will be managed directly instead of adding it to a list of Objects. To put it short I don't think there are to many apps in the wild that do it the way you brought to attention. Even though it might be a good idea to put a notice into the readme file or some similiar place that points out the change with the example you gave.

Best regards, Timo

2012/1/11 amadaden < reply@reply.github.com

I have not had a chance to extensively test this yet but from what I see the changes look good and simple. The only way I can think of that these changes could cause a problem is if someone did something really weird like if ( someObject instanceof GLSurfaceView.Renderer) That if block would now fire if someObject was a GLWallpaperService.Renderer where it would not before.


Reply to this email directly or view it on GitHub:

https://github.com/markfguerra/GLWallpaperService/pull/18#issuecomment-3450263

markfguerra commented 12 years ago

Yeah, you guys are spot on. That check would always return as true now, assuming someObject is a GLWallpaperService.Renderer. I thought about what @amadaden said but I don't really think it will be an issue for most people.

The worst I could see is that some user may be calling instanceof if they were somehow trying to accomplish the same effect as our patch in a different way. If so, they might have to make some change to their code, but this change would simplify their coding anyway because they wouldn't have to check their renderer types. The benefit outweighs this edge case in my opinion.

@Projekt2501 Yeah we should have a blurb in the release notes

markfguerra commented 12 years ago

I merged branch renderer-interface into master and released it in GLWallpaperService version 0.9.2. Thanks everyone for helping out.