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

opengl call to glDrawElements() ignores numIndices parameter on PC, but works on Android #6618

Closed
1 task
megalithic3 opened this issue Aug 6, 2021 · 7 comments

Comments

@megalithic3
Copy link

Issue details

Using opengl graphics library directly ...
import com.badlogic.gdx.graphics.GL20;
private static GL20 GLES20;
GLES20 = Gdx.gl20;

Everything works perfectly when launching to Android devices. I have an entire scene with thousands of triangles rendered just fine, but when running on PC there are extraneous triangles being drawn all over the scene. I tracked the problem down to glDrawElements(). On PC desktop launch, glDrawElements() draws incorrect number of indices, ignoring the numIndices second parameter completely.

GLES20.glDrawElements(GL20.GL_TRIANGLES, numIndices, GL20.GL_UNSIGNED_SHORT, mIndices);

For example, changing the numIndices to 3 should render only one triangle.

GLES20.glDrawElements(GL20.GL_TRIANGLES, 3, GL20.GL_UNSIGNED_SHORT, mIndices);

Here is the expected correct result on Android.

imageandroid

But the desktop launch show all triangles, ignoring numIndices completely.

imagepc

Reproduction steps/code

An SSCCE can be found at
https://gist.github.com/arashbi/5659096

NOTE: There is one bug. The mColorData needs to be changed to floating points or the shader will crash.

Version of libGDX and/or relevant dependencies

1.10.0

Stacktrace

NA

Please select the affected platforms

  • Windows 10
@mgsx-dev
Copy link
Contributor

mgsx-dev commented Aug 6, 2021

indeed, both desktop backends are impacted :
https://github.com/libgdx/libgdx/blob/master/backends/gdx-backend-lwjgl/src/com/badlogic/gdx/backends/lwjgl/LwjglGL20.java#L279
https://github.com/libgdx/libgdx/blob/master/backends/gdx-backend-lwjgl3/src/com/badlogic/gdx/backends/lwjgl3/Lwjgl3GL20.java#L278

unfortunately lwjgl doesn't provide methods for that and i don't know if we could temporarily limit the buffers.

@mgsx-dev
Copy link
Contributor

mgsx-dev commented Aug 6, 2021

Actually this is what we do in Mesh code, see https://github.com/libgdx/libgdx/blob/master/gdx/src/com/badlogic/gdx/graphics/Mesh.java#L617

so maybe we should move part of this code (limit) to desktop backends code?

@Tom-Ski do you know why we did that and if the fix proposal looks good to you?

@Tom-Ski
Copy link
Member

Tom-Ski commented Aug 6, 2021

Seems good to me.

As to why that caching and restoring is done, (if that is your question), I believe its so that the state of the index buffer isn't altered if someone is doing manipulation using the buffer itself.

@mgsx-dev
Copy link
Contributor

mgsx-dev commented Aug 6, 2021

@Tom-Ski my question was why we limit the buffer in Mesh class since we call a method that should do this job : glDrawElements (int mode, int count, int type, Buffer indices);
I was wondering if we did that because of some known backends bugs.

Anyway i have a working test case right now, i'll do the PR, thx. :-)

@mgsx-dev mgsx-dev self-assigned this Aug 6, 2021
@Tom-Ski
Copy link
Member

Tom-Ski commented Aug 6, 2021

Ah. Yeah I think this is due to the discrepancies of glDrawElements on the different platforms. We've had a few quirky issues here over time for the same reason as everyone is doing their own thing slightly differently with respect to the glDrawElements with Buffer param method.

Change that brought that behavior in is super old, so would have to check what the other platforms were doing at that time too

mgsx-dev added a commit to mgsx-dev/libgdx that referenced this issue Aug 6, 2021
mgsx-dev added a commit to mgsx-dev/libgdx that referenced this issue Aug 6, 2021
@megalithic3
Copy link
Author

megalithic3 commented Aug 7, 2021

Much appreciated folks. I'm very new to github and such, so pretty much a noob question. When the pull request is merged, I guess I need to get the entire libGDX project on to my machine and build it? Sounds daunting. Or do I wait for a release?

@tommyettinger
Copy link
Member

You can use a snapshot version, see https://github.com/libgdx/libgdx/wiki/Updating-LibGDX -- snapshots are builds of libGDX from between stable releases, usually built nightly (I'm not sure of the current schedule). The current snapshot version is 1.10.1-SNAPSHOT, so you would change the version 1.10.0 (wherever it is, most likely in the root build.gradle and/or gradle.properties) to 1.10.1-SNAPSHOT, sync the project in your IDE (or just run any Gradle task), and then the latest snapshot will be downloaded. Be aware that snapshots are built automatically without being completely checked out by a person, so sometimes bugs get into snapshots and are fixed by the next snapshot build (hopefully). You also need to change the snapshot version from 1.10.1-SNAPSHOT to 1.10.1 when the stable 1.10.1 release comes out, since the snapshot won't last very long after a release is complete.

mgsx-dev added a commit to mgsx-dev/libgdx that referenced this issue Dec 2, 2021
Quillraven pushed a commit to Quillraven/libgdx that referenced this issue Jan 2, 2022
…gdx#6621)

* fix libgdx#6618 move vertex array index buffer limit to backends

* Apply formatter

* fix test

* Apply formatter

Co-authored-by: GitHub Action <action@github.com>
SimonIT added a commit that referenced this issue Feb 8, 2023
* (#6480) load parallax factor value from Tiled

* (#6480) add parallax layers

* (#6480) add parallax render support (first draft)

* (#6480) set parallax scrolling factor

* (#6480) update parallax rendering

* (#6480) use cache boundaries for cached renderer

* (#6480) remove unused method

* (#6480) update CHANGES

* (#6480) remove unused Vector3

* (#6480) fix formatting

* (#6480) fix hierarchical scrolling factor loading

* (#6480) update parallax rendering (tile and image layer)

* (#6480) remove wrong comment

* Replace gradle deprecations (#6652)

* Upgrade gradle wrapper to 6.9

* Update junit

* Replace deprecated compile for dependencies

* Replace deprecated archiveName with archiveFileName

* Upgrade to gradle 6.9.1

* Replace compile to api in headless

* Update gdx-jnigen to 2.2.1 to fix build issues. (#6720)

* Add back breaking change notice (#6723)[CI skip]

* Update to LWJGL 3.3.0

* Revert "Temporarily comment LWJGL3 macOS ARM64 dependencies"

This reverts commit 7ac239c.

* Update fetch.xml

* fix bad camera reference by using instance variable instead of parameter (#6722)

* Javadocs to make it clear Pool doesn't reset if discarding.

* Revert "Fixed texture packer legacy output having wrong Y value when using padding."

This reverts commit 08cd7fd. Turns out the "fix" was actually a bug!

closes #6701

* fix #6618 move vertex array index buffer limit to backends (#6621)

* fix #6618 move vertex array index buffer limit to backends

* Apply formatter

* fix test

* Apply formatter

Co-authored-by: GitHub Action <action@github.com>

* Add missing .gitignore to gdx-bullet

* Lwjgl3Net: Fallback to xdg-open on Linux if Desktop.BROWSE is unavailable (#6719)

* Desktop.Action.BROWSE is unavailable inside of Linux sandboxes. We're expected to use xdg portals where available

* add back default Pool behavior when object is discarded (#6730)

* Remove all usage of Arial assets, replace with LSans. (#6727)

* Fixes Gradle build issues (GWT Tests, Runnable Tools) (#6731)

* Let buildRunnables task depend on build task so all dependencies are present

* gwtref: Throw ClassNotFoundException instead of RuntimeException in ReflectionCache.forName (#6718)

* gwtref: Throw ClassNotFoundException instead of RuntimeException in ReflectionCache.forName

Fixes #6708

* gwt/emu/reflect: Add exception cause in ClassReflection.forName

* Gradle publishing support (#6725)

* Should match maven for artifacts, including `$NAME-platform` artifacts for jnigen natives.

* Clean Up PixmapBlendingTest (#6732)

This may fix an unclear issue with PixmapBlendingTest, as well, but in general it makes the code more readable and idiomatic for a libGDX test.

* Use gradle for github-actions publishing

* javadoc encoding also, remove group from box2d parent

* Update build status badge in README

* Update freetype to v2.10.4 and use submodule.

* Update freetype to v2.11.1

* Populate cppExcludes to prevent build issues in gdx-freetype

* More specific cppExcludes to make android happy

* Add note about breaking change on legacy LWJGL3 projects.

* (#6480) update CHANGES

* (#6480) run spotlessApply on changed map files

* Modified the math in all map renderers to subtract on the x value to the parallax effect works properly.
Modified test maps parallax value so it was easier to see the effect.
Made a change in the BaseTmxMapLoader just to keep it consistent with the rest of the code.

* Changed OrthoCachedTiledMapRenderer to use negative instead of positive like the rest, also changed using cacheBounds to determine parallax to using viewBounds like the rest.
Also modified the ortho map to have a non group parallax layer. so it will show up in OrthoCachetest

* Apply formatter

---------

Co-authored-by: klausm <simon.klausner@swarovski.com>
Co-authored-by: SimonIT <simonit.orig@gmail.com>
Co-authored-by: PokeMMO <2398581+PokeMMO@users.noreply.github.com>
Co-authored-by: MGSX <germain.aubert@gmail.com>
Co-authored-by: Desu <desu@pokemmo.eu>
Co-authored-by: Nathan Sweet <nathan.sweet@gmail.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Kyu <kyu@pokemmo.eu>
Co-authored-by: Tommy Ettinger <tommy.ettinger@gmail.com>
Co-authored-by: Justin Shapcott <support@mobidevelop.com>
Co-authored-by: Justin Shapcott <justin@mobidevelop.com>
Co-authored-by: nojus297 <46261165+nojus297@users.noreply.github.com>
Co-authored-by: Yuri Pourre <yuripourre@users.noreply.github.com>
Co-authored-by: Tomski <tomwojciechowski@asidik.com>
Co-authored-by: BoBIsHere86 <rck86@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants