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

Fixed images in black margins #37475

Merged
merged 1 commit into from Jul 3, 2020
Merged

Fixed images in black margins #37475

merged 1 commit into from Jul 3, 2020

Conversation

azagaya
Copy link
Contributor

@azagaya azagaya commented Mar 31, 2020

Fixes #27440. Also, i could add the option so stretch image to margin size, but would require new methods to set that option.

@azagaya azagaya requested a review from reduz as a code owner March 31, 2020 22:13
@azagaya
Copy link
Contributor Author

azagaya commented Mar 31, 2020

Oops... i added a white space at the beginning. Let me fix it.

@YeldhamDev YeldhamDev added bug cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:rendering labels Apr 1, 2020
@YeldhamDev YeldhamDev added this to the 4.0 milestone Apr 1, 2020
@YeldhamDev YeldhamDev removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Apr 1, 2020
@YeldhamDev YeldhamDev modified the milestones: 4.0, 3.2 Apr 1, 2020
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@akien-mga
Copy link
Member

Could you squash the two commits into one?

@azagaya
Copy link
Contributor Author

azagaya commented Jun 3, 2020

Oh, sorry... I think it should be fine now.

@@ -250,7 +250,8 @@ void RasterizerCanvasBaseGLES2::draw_window_margins(int *black_margin, RID *blac
if (black_image[MARGIN_LEFT].is_valid()) {
_bind_canvas_texture(black_image[MARGIN_LEFT], RID());
Size2 sz(storage->texture_get_width(black_image[MARGIN_LEFT]), storage->texture_get_height(black_image[MARGIN_LEFT]));
draw_generic_textured_rect(Rect2(0, 0, black_margin[MARGIN_LEFT], window_h), Rect2(0, 0, sz.x, sz.y));
draw_generic_textured_rect(Rect2(0, 0, black_margin[MARGIN_LEFT], window_h),
Rect2(0, 0, (float)black_margin[MARGIN_LEFT] / sz.x, (float)(window_h) / sz.y));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we confident that sz.x and sz.y can't ever be 0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with this and it did not crash nor return any error, so I guess there's some validation that happens further up the stack:

	var rid = VisualServer.texture_create()
	print(rid.get_id())  # 147
	print(VisualServer.texture_get_height(rid))  # 0
	print(VisualServer.texture_get_width(rid))  # 0
	
	VisualServer.black_bars_set_images(rid, rid, rid, rid)
	VisualServer.black_bars_set_margins(50,50,50,50)

@akien-mga
Copy link
Member

Tested locally and it does fix the bug, and works fairly well. I've noticed some weird artifacts when resizing but this might be due to Linux/KWin issues. I'd be interested in a test project as mentioned in #27440 to see if I can confirm that issue and open a bug report.

@akien-mga akien-mga merged commit 2286336 into godotengine:3.2 Jul 3, 2020
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants