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

Black Margins and Black Margins Images not working properly #40135

Closed
azagaya opened this issue Jul 5, 2020 · 13 comments
Closed

Black Margins and Black Margins Images not working properly #40135

azagaya opened this issue Jul 5, 2020 · 13 comments
Assignees
Milestone

Comments

@azagaya
Copy link
Contributor

azagaya commented Jul 5, 2020

Godot version:
Tested in 3.2.2 stable, 3.1.2 stable and on custom build of 3.2 branch (commit 1ebfe84).

OS/device including version:
Tested ib Xubuntu 20.04, kernel 5.4.0-40-generic

Issue description:
Setting textures to black bars in margins generates some artifacts when scaling. Also, i noticed in my notebook that using the same texture for the black bars images inside a scene, causes the margins become completely white. On 3.2 branch from the repo, the scenes WorkingExample and SimpleExample should correctly show the images in the margins due to #37475. White bars still apears in both 3.2 branch and 3.2.2 stable version as far as i tested. The white margins issue only happens in GLES3.

Steps to reproduce:
Open the example project attached. It contains three examples. You can see on them both issues described above. Just run the differents scenes and test.

Minimal reproduction project:
BlackBarasTest.zip

@Calinou
Copy link
Member

Calinou commented Jul 5, 2020

Did this work in a previous version like Godot 3.1? Also, have you tested both GLES3 and GLES2 renderers?

@azagaya
Copy link
Contributor Author

azagaya commented Jul 5, 2020

@Calinou I can confirm it also happens in 3.1.2. Also, i now tested with gles2, and it seems not to present the white margins issue, although the glitch when resizing is still there. I updated the issue with that info.

@akien-mga akien-mga added this to the 3.2 milestone Jul 5, 2020
@lawnjelly
Copy link
Member

Ok took me a while to work out but for the benefit of anyone else:

  • azagaya made a recent change to fix the texture coordinates of textures applied as 'black margins'
  • in order to test this you need to build the latest 3.2, i.e. after the PR mentioned

The reason you are seeing glitches (at least what I think you are referring to) is probably an order of operations or double / triple buffering issue. This is fairly common with viewport resizing.

The order of operations should be:

  1. Resize viewport dimensions
  2. Determine positions etc of everything on screen
  3. Draw.

It is easy to end up in a situation where it is:

  1. Determine positions etc of everything on screen
  2. Resize viewport dimensions
  3. Draw

Which will result in things drawing in the wrong places, areas not drawn outside the old viewport area (maybe this is the white) etc.

Also you can potentially get problems from double / triple buffering, if the displayed image is from a frame that was created before the resize took place.

To solve this first of all I'd turn off double buffering etc if possible to rule that out, then you maybe look at the timing at which resizes take place. You might be able to force them to take place at a certain point by recording the resize event from the OS, not apply it immediately, and apply at the correct point in time.

There also could be something somewhere in the code introducing a frame lag with the viewport dimensions.

If you are getting problems from the double buffering, then you may be able to cause a double screen update on a resize event.

@azagaya
Copy link
Contributor Author

azagaya commented Jul 6, 2020

Hi! Just one question:

Which will result in things drawing in the wrong places, areas not drawn outside the old viewport area (maybe this is the white) etc.

Could it be possible white margins be caused for something else? In the scene in the test project where the brick texture is both used for black margin image, and added as a sprite node to the scene (NotWorkingExample1.tscn), margins turn completely white. But just changing that sprite to another, it starts "working". I only noticed it on GLES3

@lawnjelly
Copy link
Member

Sure, I can only report what I saw. Really needs a screenshot or video for us to see. It could be that a state change means the GL state is incorrect in some circumstances (maybe the draw_margins command is assuming a particular GL state).

@azagaya
Copy link
Contributor Author

azagaya commented Jul 6, 2020

Here i leave a video where margins turning white when the same texture is used in the scene and for black margins image.
ezgif com-video-to-gif(7)

@lawnjelly
Copy link
Member

Ah yes that does look like a state is incorrect when the texture has already been used. Or it thinks it is still bound, so isn't binding inside the draw_margins call.

@azagaya
Copy link
Contributor Author

azagaya commented Jul 6, 2020

Ah, ok. Is this suitable for a junior? Where should i start looking to fix this two issues?

@lawnjelly
Copy link
Member

Ah, ok. Is this suitable for a junior? Where should i start looking to fix this two issues?

The white texture sure, if you are familiar with OpenGL. Presumably in that draw_margins function. The glitches on resize might be trickier but there is nothing to stop you investigating.

@akien-mga
Copy link
Member

Is this fully or only partly fixed by #40169?

@lawnjelly
Copy link
Member

lawnjelly commented Jul 8, 2020

Only partially as I understand it. However the glitchy behaviour on resize may be a more general problem, not specific to margins (maybe there is another issue mentioning this already?). So if this remains open it could do with renaming (or better still a new issue creating. 1 issue one bug).

@azagaya
Copy link
Contributor Author

azagaya commented Jul 11, 2020

Should we close this and open another issue for the glitch? I didnt find any other issue about it.

@akien-mga
Copy link
Member

Yeah that would make sense. Let's close as fixed by #40169 then.

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

5 participants