Slick Forums

Discuss the Slick 2D Library
It is currently Sun Nov 19, 2017 2:42 am

All times are UTC




Post new topic Reply to topic  [ 63 posts ]  Go to page 1, 2, 3, 4, 5  Next
Author Message
PostPosted: Sat Mar 10, 2012 5:02 am 
I've added Managed Shader support in the package org.newdawn.slick.shader. To create a test class, I need an example vertex and fragment shader. Anyone?


Top
  
 
PostPosted: Sat Mar 10, 2012 8:38 am 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
Hey, I am working on something similar so maybe we should combine our efforts.

Sorry, but I have some issues with your current system:
  • Every time you set a uniform value, it needs to query the uniform location first. This could potentially lead to a performance loss since most shaders will require you to update multiple uniforms per frame.
  • getGLVersion shouldn't be a public method of ShaderManager -- (a) because it doesn't return the expected verison (but instead just one digit) and (b) because, from an object-oriented point of view, it doesn't belong in a class about shaders
  • You should check for versions via the GLContext.getCapabilties().OpenGLXX booleans, and shader support with the GL_ARB_shader_objects (and vertex/frag objects) boolean also in ContextCapabilities
  • Right now there is a lot of duplicate code since you use GL20 and ARBShaderObjects. Ideally you should stick to one; if the version is >= 2.0 then the shader extension (ARBShaderObjects) should work regardless. If we want the slight benefit that GL20 code might provide then we should be implementing that in our SGL/Renderer classes.
  • You should use Slick's Log instead of System.out.print
  • Your programs don't have any error checking on the compiling/source code, so there's no way for the user to know whether there was a problem
  • All of your code could easily fit into a single class; and IMO it should be renamed to ShaderProgram. Multiple shaders (vertex, fragment, geometry) might link to a single OpenGL program

Great to see interest in shaders, though!


Top
 Profile  
 
PostPosted: Sat Mar 10, 2012 10:38 am 
Quote:
Every time you set a uniform value, it needs to query the uniform location first. This could potentially lead to a performance loss since most shaders will require you to update multiple uniforms per frame.


Thanks, I'd never thought of this issue. Obviously a biggie. How would this be solved? Would it be more/less efficient to have a map resolving variables to IDs?

Quote:
getGLVersion shouldn't be a public method of ShaderManager -- (a) because it doesn't return the expected verison (but instead just one digit) and (b) because, from an object-oriented point of view, it doesn't belong in a class about shaders


Understood.

Quote:
You should check for versions via the GLContext.getCapabilties().OpenGLXX booleans, and shader support with the GL_ARB_shader_objects (and vertex/frag objects) boolean also in ContextCapabilities


Done

I'm currently refining this class again. Currently I've got an abstract class called Program, which has (along with other stuff) a load method which returns a Shader. 2 classes extend this, ARBProgram and GLProgram, which both have their own implementations of their methods. This way no class is checking if it is ARB or OpenGL2+. I'm also investigating into supporting Geometry Shaders, however this would require a whole new level checking since it's only supported on OpenGL3.2+.

Quote:
All of your code could easily fit into a single class; and IMO it should be renamed to ShaderProgram. Multiple shaders (vertex, fragment, geometry) might link to a single OpenGL program


Also done. I'm thinking I could have a Program class, and if you wanted to say use a geometry shader, you could call a method attachShader(Type.geometry, source). How does this sound?


Top
  
 
PostPosted: Sat Mar 10, 2012 2:59 pm 
Offline
Game Developer
User avatar

Joined: Thu Mar 03, 2011 6:22 pm
Posts: 534
liam, please stopp getting ahead on everything. I have a lot of problems with your system too. davedes system is also a lot cleaner and easier to understand imho :D

_________________
Current Projects:
Image Mr. Hat I
Image Vegan Vs. Zombies
Projects:
RadicalFish Engine - Build on top of Slick2D, Ideas, Bugs? Open an Issue ticket!


Top
 Profile  
 
PostPosted: Sat Mar 10, 2012 3:33 pm 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
LibGDX maps the uniform locations to a hash map, which makes it pretty easy to call things like this:
Code:
shaderProgram.setUniform3f("blah", 2f, 2f, 2f);


I'd imagine it would be faster than asking GL to lookup the integer each time. The best way to do it in terms of sheer performance would be to let the user decide how to map the IDs -- i.e. give them a getUniformID(String) method that returns an int, then let them pass an ID using setUniform2f(int, float, float). This is something with already do with Sound and the play method, which returns an integer ID.

In terms of the ARB thing, just do it like so:
  • Add methods glCreateProgram/glGetUniformLocation/etc to SGL (this will break compatibility, but hopefully any user trying to extend SGL should realize that is is unstable and will be frequently changed to include new Slick features!)
  • In ImmediateModeOGL, implement the methods via ARBShaderObjects and constants via GL20 (as they map to the same value)
  • Call these abstracted methods from the ShaderProgram class
  • At a later point, we may decide to further abstract our rendering so that the SGL implementation decides whether to use ARB/EXT or GLXX.

Like I said, if GL20 is supported then ARBShaderObjects should also work. There are some slight benefits to using the GLXX classes instead if it's supported, which is why we should eventually abstract it (though it's not a huge priority). This also goes for FBO, PBuffer, and anything else in Slick that uses EXT/ARB.

EDIT:
Also, it makes sense to move the package into opengl -- i.e. "org.newdawn.slick.opengl.shader"


Top
 Profile  
 
PostPosted: Sat Mar 10, 2012 7:33 pm 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
Here's some changes to my old class. It uses a hash map for convenience, like LibGDX, and adds a Shader inner class (which maybe will be moved to outer leve) for shader/program independency.
http://pastebin.com/kSXpYnAb

The releasing stuff is a bit messy. We'll have to add things like detachShader/attachShader if we really want it to be independent (and make shaders reusable over multiple programs).


Top
 Profile  
 
PostPosted: Sun Mar 11, 2012 12:50 am 
NOTE: Code has an error currently, being fixed

Take a look at a more efficient version of my shader class here. I've implemented alot of the things mentioned in this thread (except for error checking, todo still). It uses a HashMap for efficient variable lookup. The variables are loaded into the HashMap when a shader is attached. The attachShader function allows for easy attaching of different types of shaders, and doesn't restrict the program to using a fragment and vertex shader. Currently I still have to implement geometry shader support.

Another efficient part of this code in comparison to my last implementation, is that it isn't using if statements everywhere to check if it is a GL20 or ARB shader. All of this works with ARB and GL20.

An example usage -

Code:
Program program = Program.create(); // Depending on the platform, will create either an ARB or GL20 program
program.attachShader(ShaderType.VERTEX, "vertexshadersourcehere");
program.finaliseProgram();

//Later
program.enable();
program.disable();


Top
  
 
PostPosted: Sun Mar 11, 2012 2:11 am 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
Instead of abstracting the entry point to GL in the Program class, it should be done in Slick's SGL class. (Same goes for FBOGraphics.)


Top
 Profile  
 
PostPosted: Mon Mar 19, 2012 10:00 am 
Offline
Game Developer
User avatar

Joined: Thu Mar 03, 2011 6:22 pm
Posts: 534
So can we take this into the dev branch? I would like to experiment a bit with shaders (especially providing some simple shaders)

Some Questions:
  • Shoulda specifiv Shader class implement methods like update to update the uniforms and so on?
  • What is the benifit in naming it "Program" instead of ShaderProgram?
  • Error locking would be cool, I suggest throwing the developer out of the game not spaming error messages?
  • You might want to use the ResourceLoader class for the source String :)

Edit:
Also, on line 145. You try to get the length of an array, can you be sure the objuect is not null?
Because I get a null pointer with no information at all :/ I guess because you forgot to load the source but expect to user to load it :o I would implement functions for that like davedes did.

_________________
Current Projects:
Image Mr. Hat I
Image Vegan Vs. Zombies
Projects:
RadicalFish Engine - Build on top of Slick2D, Ideas, Bugs? Open an Issue ticket!


Top
 Profile  
 
PostPosted: Mon Mar 19, 2012 10:22 am 
@R.D. Sorry man, should've removed the code earlier, I forgot. It's not stable right now, plus it's also not efficient. My newer implementation is better (however still doesn't work :( ). I'll push to my fork later this week when I have some time.

Quote:
What is the benefit in naming it "Program" instead of ShaderProgram?


Because it's the proper name.


Top
  
 
PostPosted: Mon Mar 19, 2012 12:02 pm 
Offline
Game Developer
User avatar

Joined: Thu Mar 03, 2011 6:22 pm
Posts: 534
Maybe you really want to look and davdes implementation. I got it working without even reading the source or anything :) Want you want for sure is loading the source into a string using the ResourceLoader! That's the proper way for Slick2D loading.

Ah okay, I wasn't aware of it :D Keep on doing awesome stuff :D

_________________
Current Projects:
Image Mr. Hat I
Image Vegan Vs. Zombies
Projects:
RadicalFish Engine - Build on top of Slick2D, Ideas, Bugs? Open an Issue ticket!


Top
 Profile  
 
PostPosted: Mon Mar 19, 2012 2:03 pm 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
Here's some things to consider:
  • There is no need for an update() method as far as I know.
  • If we want to allow users to attach the same Shader to multiple Programs (which is not a very common requirement for simple games -- LibGDX doesn't support this, for example), then we need to tweak things (hence why I started writing an object-oriented Shader class along with ShaderProgram). We also need to take things like releasing() and possibly allowing for geometry shaders (which IMO is overkill for a 2D library).
  • If we don't require the above feature then the class will be much more minimal (more like my first draft).
  • We need to include all setUniform1f/setUniform2f/etc. as well as the same for setAttribute. This might be more "clutter" than simply including a method that allows for a float, but it will be more efficient for the end user.
  • In order to make it more Slick-friendly, we can introduce getUniform1f/getUniform2f/etc. -- noting in JavaDoc that it needs to create a new byte buffer per call (so should only really be used for convenience).
  • Using multiple textures (glActiveTexture) is not very slick-friendly at the moment, we'll have to figure something out with that. But many shaders only require one texture, which will be active by default AFAIK

Otherwise the class I posted works fine and is ready to at least be placed into the dev branch. I've been using it with my own game now with no problems.


Top
 Profile  
 
PostPosted: Mon Mar 19, 2012 3:45 pm 
Offline
Game Developer
User avatar

Joined: Thu Mar 03, 2011 6:22 pm
Posts: 534
I'm already working with you version although I can't seem to get a uniform to fit in. I just added a unfiform float to my fragment shader and for some reason it does not pop up in the uniform HashMap.

This is the shader.
Code:

uniform float time;
uniform sampler2D sampler0;

void main() {   
   gl_FragColor = texture2D(sampler0, gl_TexCoord[0].st);
}

_________________
Current Projects:
Image Mr. Hat I
Image Vegan Vs. Zombies
Projects:
RadicalFish Engine - Build on top of Slick2D, Ideas, Bugs? Open an Issue ticket!


Top
 Profile  
 
PostPosted: Mon Mar 19, 2012 7:13 pm 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
Uniforms are only "active" in GLSL if the compiler can determine that they are actually used; so "time" (since you don't use it in the shader) isn't considered an active uniform and thus its uniform location (i.e. ID) returns -1.
http://ogltotd.blogspot.ca/2007/12/acti ... ertex.html

I'll have to implement something to ShaderProgram like hasUniform or isUniformActive, as well as some error checking within setUniformXX to ensure that no null pointers are thrown.

Thanks for the find.


Top
 Profile  
 
PostPosted: Mon Mar 19, 2012 8:14 pm 
Offline
Slick Zombie

Joined: Sat Jan 27, 2007 7:10 pm
Posts: 1482
Update:
http://pastebin.com/sDxaFMh9

"Strict mode" is by default enabled; this means that trying to use setUniform1f on a non-active uniform will throw an illegal argument exception. You can disable strict mode to have these exceptions silently suppressed (i.e. so that altering a non-active uniform does nothing).

Also, a simple shader test that inverts the colors of a texture:
http://pastebin.com/3333UaYv

Vert:
Code:
void main() {
   gl_TexCoord[0] = gl_MultiTexCoord0;
   gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex;
}


Frag:
Code:
uniform sampler2D tex;

void main() {
   vec4 color = texture2D(tex, gl_TexCoord[0].st);
   gl_FragColor = vec4(1.0 - color.r, 1.0 - color.g, 1.0 - color.b, 1);
}


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 63 posts ]  Go to page 1, 2, 3, 4, 5  Next

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group