Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should probably add the methods to the interface... #1398

Closed
wants to merge 1 commit into from
Closed

Should probably add the methods to the interface... #1398

wants to merge 1 commit into from

Conversation

Pesegato
Copy link
Contributor

...and fix also android etc. but still, at least we do have that information for desktop.

...and fix also android etc. but still, at least we do have that information for desktop.
@pspeed42
Copy link
Contributor

This is showing the OpenCL info... and only for the last device listed.

For display driver info that is useful for graphics settings screens and such, it would be the stuff in printContextInitInfo().

Also, without a change to the interface then this still requires a cast to use... so the caller might as well just import Display directly.

@Pesegato
Copy link
Contributor Author

printContextInitInfo() don't have the "profile" information.

The OpenCL part is the ONLY place where "profile" is printed. I agree this is bad design and hackish, but at least is a step that makes it possible to see the opengl profile.

@pspeed42
Copy link
Contributor

It's already possible... by going directly to lwjgl. This code is currently no better than that.

@Pesegato
Copy link
Contributor Author

Please show me where it is done. I haven't found a method, and the only this I've found in JME is that initialization code that (I think) shouldn't be invoked from the application.

@pspeed42
Copy link
Contributor

Nothing stops an application from calling "CLPlatform.getPlatforms()" itself and looking at the information.

The whole point of having this support in JME would be that you wouldn't have to go directly to platform specific code. If the support is only on the platform specific context anyway then there is no point... because the application could just go to lwjgl directly since they'd have already tied themselves directly to that.

@Pesegato
Copy link
Contributor Author

Pesegato commented Sep 25, 2020

Yes, I can call getPlatform but:

new LwjglPlatform(CLPlatform.getPlatforms().get(0)).getDevices().get(0).getProfile()

gives me a nice:

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x0000000000000000, pid=9264, tid=3280

and if I try

((LwjglDevice)CLPlatform.getPlatforms().get(0).getDevices(0).get(0)).getProfile()

incompatible types: CLDevice cannot be converted to LwjglDevice

and of course

incompatible types: CLPlatform cannot be converted to LwjglPlatform

@pspeed42
Copy link
Contributor

And you are doing this on the render thread?

I mean, apparently the JME code works. It shouldn't care what class it's in I guess.

@Pesegato
Copy link
Contributor Author

Pesegato commented Sep 29, 2020

At first it was on the initialize() of an AppState, now I've tried with this

    public static void main(String[] args) throws Exception {

        System.out.println("**************** profile "+new LwjglPlatform(CLPlatform.getPlatforms().get(0)).getDevices().get(0).getProfile());

Still got the crash, and the profile is not printed. I take that unless you are a JME guru you just can't access this API.

@Pesegato
Copy link
Contributor Author

...and this is starting to look ridiculous.
Please make me look like an idiot and provide a one line that actually prints the profile (what I was not able to do).

@pspeed42
Copy link
Contributor

Seems like you are doing that before the context is initialized and not on the render thread. Even the code you added to JME cannot be used before the context has been initialized.

@Pesegato
Copy link
Contributor Author

Pesegato commented Sep 29, 2020

Ok, I've waited until the context is initialized (graphics and mouse works). Then the debugger tells me that the thread is "jME3 Main".
Still got the crash.
So? Should I provide a sample test app?

@pspeed42
Copy link
Contributor

pspeed42 commented Sep 29, 2020

So I guess the same JME code that you added has magic fairy dust. I can't explain why the same code works in one place and not another.

...though it does make me wonder if the code added to JME will randomly break at different times now.

@Pesegato
Copy link
Contributor Author

import com.jme3.app.SimpleApplication;
import com.jme3.opencl.lwjgl.LwjglPlatform;
import com.jme3.system.AppSettings;
import org.lwjgl.opencl.CLPlatform;

public class HelloMagicFairyDust extends SimpleApplication{

    public static void main(String[] args) {
        HelloMagicFairyDust app=new HelloMagicFairyDust();
        app.setShowSettings(false);
        AppSettings settings=new AppSettings(true);
        app.setSettings(settings);
        app.start(); // start the game
    }

    @Override
    public void simpleInitApp() {

    }

    @Override
    public void simpleUpdate(float tpf){
        System.out.println(" Profile "+ new LwjglPlatform(CLPlatform.getPlatforms().get(0)).getDevices().get(0).getProfile());
    }
}

@Pesegato
Copy link
Contributor Author

Please try to run the above

@tlf30
Copy link
Contributor

tlf30 commented Sep 29, 2020

Oh, I recall something about this when trying to do the same thing.... I can't remember, but perhaps LWJGL Platform can only be created once per context.... I will setup a test to see what I can find.

@MeFisto94
Copy link
Member

tbh you are on a completely wrong approach.
IMO you should check what information lwjgl2, lwjgl3 and jogl expose and design an interface that could more or less be satisfied by those three.

Then the tricky question is: How can we expose this API programmatically without having to depend on the lwjgl2, 3 or jogl modules. Not sure if you can, but either way the API should be around the same and you could just save the info during context init (it won't be available earlier than that anyway) and then just query that "store" where you put them into.

I guess funnier things are how to handle multi gpu setups, when lwjgl2 probably doesn't support that or other advanced info like GL Features, that jme only queries for one device in the long urn.

TLDR: Design an Interface, Implement it directly where the log is printed.

@Pesegato
Copy link
Contributor Author

Pesegato commented Oct 8, 2020

So, anybody tried to run and experience the crash with my code?

@pspeed42
Copy link
Contributor

pspeed42 commented Oct 8, 2020

I haven't, personally.

In the end, this PR is probably not going to be accepted. Once we add public methods they are part of the public API essentially forever. If someone came along next week and did a real and proper implementation looking at the possible settings across all of the supported platforms and then decided the methods in this PR wouldn't fit that then we end up dragging around useless methods for several years.

So my vote is that we wait for the proper implementation that adds methods to the interface and support for the other platforms.

@Pesegato Pesegato closed this Oct 8, 2020
@Pesegato Pesegato deleted the patch-1 branch October 13, 2020 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants