godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.31k stars 20.23k forks source link

Batching+Modulate issue for multimesh drawn in _draw with multiple modulated sprites before it #51883

Closed raymoo closed 3 years ago

raymoo commented 3 years ago

Godot version

3.3.stable

System information

Linux, GLES3, GTX 750Ti (nouveau)

Issue description

An issue similar to #48122 but between two sprites and a Node2D using _draw to draw a MultiMesh. If I understand correctly, the original issue is supposed to have been fixed already (cherry picked to 3.3 so should affect me).

Setup: 2021-08-19-063440_242x137_scrot

The script:

extends Node2D

onready var mesh = QuadMesh.new()
onready var multimesh = MultiMesh.new()

func _ready():
    mesh.set_size(Vector2(2,2))
    multimesh.mesh = mesh
    multimesh.transform_format = MultiMesh.TRANSFORM_2D
    multimesh.color_format = MultiMesh.COLOR_8BIT
    multimesh.custom_data_format = MultiMesh.CUSTOM_DATA_8BIT
    multimesh.instance_count = 100
    multimesh.visible_instance_count = 100

    for i in range(100):
        multimesh.set_instance_transform_2d(i, Transform2D(Vector2(100,0), Vector2(0, 100), Vector2(randf()*1920, randf()*1080)))
        multimesh.set_instance_color(i, Color(1.0, 1.0,1.0))
        print(multimesh.get_instance_transform_2d(i).get_scale())

func _draw():
    draw_multimesh(multimesh, preload("res://icon.png"))

Expected output: Many icon.pngs are rendered normally.

Actual output: The icon.pngs receive the modulate of the sprites above them.

It works fine on GLES2 or if batching is disabled.

Steps to reproduce

Run the reproduction project under GLES3 with batching enabled.

Minimal reproduction project

modbatch.zip

raymoo commented 3 years ago

Not sure whether it needs to be in _draw or if it needs to be a MultiMesh, but it doesn't seem to happen if I replace the multi mesh drawer with a Sprite. I am just making a setup similar to one I experienced in my actual project.

lawnjelly commented 3 years ago

I'll take a look. :+1: Interesting that it works fine in GLES2, suggests it might be a state or something simple that is out of sync.

lawnjelly commented 3 years ago

This is already fixed in 3.x.

Be sure to try the latest version, you say you are using 3.3 stable, which is the same version as the issue you linked reported the issue in (so it is not surprising you see the same bug). Try the latest 3.3 (3.3.3) or failing that 3.4 beta 4.

EDIT: Turns out it is fixed in 3.x branch (i.e. 3.4) but not working in 3.3.3, maybe one of the fixes was not cherry picked. I'll try and work out which.

This may need @akien-mga help (maybe when back from holiday) as I'm not that much of a git expert for finding which PRs have been cherry picked to 3.3. Pretty much all the batching PRs should have been ok to cherry pick afaik, so maybe we missed one.

raymoo commented 3 years ago

3.3.stable at the time was 3.3

akien-mga commented 3 years ago

This may need @akien-mga help (maybe when back from holiday) as I'm not that much of a git expert for finding which PRs have been cherry picked to 3.3. Pretty much all the batching PRs should have been ok to cherry pick afaik, so maybe we missed one.

You can filter out PRs which have been cherry-picked by searching for NOT "Cherry-picked for <version>." since I always make sure to use this exact sentence.

Here's the list of PRs in 3.4 milestone which you made and which haven't been cherry-picked for any 3.3.x release. I don't think anything batching related: https://github.com/godotengine/godot/pulls?q=is%3Apr+author%3Alawnjelly+NOT+%22Cherry-picked+for+3.3.1.%22+NOT+%22Cherry-picked+for+3.3.2.%22+NOT+%22Cherry-picked+for+3.3.3.%22+is%3Aclosed+milestone%3A3.4

There's of course always the option that I might have commented "Cherry-picked for 3.3.3." on a PR but actually forgot to do the cherry-pick locally. The easiest way to find out would be to identify again which PR fixed it in 3.x so that we can see if it was cherry-picked or not.

lawnjelly commented 3 years ago

Maybe I can also just checkout the relevant files and do a diff to track it down, that may end up being easier. But I'll try this search! :+1: Also this could have been fixed by something outside the batching, given that it looks like possibly a GL state bug (and is GLES3 only).

akien-mga commented 3 years ago

Yeah if you can identify what's the difference between the two branches' relevant code, git blame should tell you the commit.

lawnjelly commented 3 years ago

Got it I think. Line 869 rasterizer_canvas_gles3.cpp, there's a missing:

glVertexAttrib4f(VS::ARRAY_COLOR, 1.0, 1.0, 1.0, 1.0);

In the 3.3 version. I remember fixing this, having to find which PR is was.

47582

And it's one of clayjohn's, that's why we didn't spot it probably. So this PR just needs cherry picking I think. @akien-mga (I haven't confirmed this is the fix, but it's pretty likely as it is a GL state bug with multimeshes.)

akien-mga commented 3 years ago

Thanks, I cherry-picked it for 3.3.4, so this can be closed.