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.

Sign up for GitHub

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

Jump to bottom

Add `Image::fill_rect` method #52456

Merged

akien-mga merged 2 commits into godotengine:master from kleonc:image-fill-rect

Nov 24, 2021

Member

kleonc commented Sep 7, 2021

Resolves godotengine/godot-proposals#3259.

Made the p_rect parameter Rect2 instead of Rect2i to make it consistent with other Image's methods like blit_rect(), blend_rect() etc. (if changing Rect2 to Rect2i is wanted it can be made in a separate PR).

kleonc requested a review from a team as a code owner

September 7, 2021 08:40

kleonc mentioned this pull request

[3.x] Add Image::fill_rect method #52457

Merged

kleonc force-pushed the image-fill-rect branch 2 times, most recently from 7f6d7d3 to 905567a Compare

September 7, 2021 09:20

kleonc requested a review from a team as a code owner

September 7, 2021 09:20

AaronRecord reviewed

View reviewed changes

core/io/image.cpp Outdated Show resolved Hide resolved

AaronRecord reviewed

View reviewed changes

core/io/image.cpp Outdated Show resolved Hide resolved

AaronRecord reviewed

View reviewed changes

doc/classes/Image.xml Outdated Show resolved Hide resolved

YeldhamDev added enhancement topic:core labels

YeldhamDev modified the milestones: 3.4, 4.0

AaronRecord reviewed

View reviewed changes

core/io/image.cpp Outdated Show resolved Hide resolved

kleonc force-pushed the image-fill-rect branch from 905567a to 9377345 Compare

September 8, 2021 10:18

Member Author

kleonc commented Sep 8, 2021

I've added a second commit changing these methods to use memcpy(), as @LightningAA suggested. Didn't squash it for now for easier comparison or in case such change is not wanted.

kleonc force-pushed the image-fill-rect branch from 9377345 to 96cc72b Compare

September 8, 2021 10:33

AaronRecord reviewed

View reviewed changes

Contributor

AaronRecord left a comment

Looks pretty good to me, just one knit pick.

core/io/image.cpp Outdated Show resolved Hide resolved

kleonc force-pushed the image-fill-rect branch from 96cc72b to 4292de2 Compare

September 8, 2021 15:40

AaronRecord reviewed

View reviewed changes

core/io/image.cpp Outdated Show resolved Hide resolved

kleonc force-pushed the image-fill-rect branch from 4292de2 to 5273116 Compare

September 8, 2021 19:09

AaronRecord approved these changes

View reviewed changes

Contributor

AaronRecord left a comment

Tested using the 3.x version (my computer doesn't support vulkan), everything seemed to work fine, no obvious regressions. Looks good to me 😃

AaronRecord mentioned this pull request

Correct Vector2 and Rect2 to Rect2i and Vector2i in many cases #52502

Closed

AaronRecord suggested changes

View reviewed changes

Contributor

AaronRecord left a comment •

edited

Loading

Actually, just remembered one more small thing (sorry), it might be a good idea to add some tests for fill_rect(...) in tests/test_image.h. There's already a test for fill(...)

kleonc force-pushed the image-fill-rect branch from 5273116 to 9967ab6 Compare

September 10, 2021 21:29

kleonc requested a review from a team as a code owner

September 10, 2021 21:29

Member Author

kleonc commented Sep 10, 2021

Actually, just remembered one more small thing (sorry), it might be a good idea to add some tests for fill_rect(...) in tests/test_image.h. There's already a test for fill(...)

Done.

AaronRecord reviewed

View reviewed changes

tests/test_image.h Outdated Show resolved Hide resolved

AaronRecord approved these changes

View reviewed changes

kleonc force-pushed the image-fill-rect branch from 9967ab6 to 245b3b4 Compare

October 5, 2021 18:02

akien-mga approved these changes

View reviewed changes

Member

akien-mga commented Nov 23, 2021

Needs a rebase.

kleonc and others added 2 commits

November 23, 2021 18:06


          Add Image::fill_rect method

91595b1


          Image Use memcpy() in fill(), fill_rect(), _put_pixelb(), _get_pixelb()

92ca300

Co-authored-by: Lightning_A <aaronjrecord@gmail.com>

kleonc force-pushed the image-fill-rect branch from 245b3b4 to 92ca300 Compare

November 23, 2021 17:11

akien-mga merged commit b1bf82d into godotengine:master

Member

akien-mga commented Nov 24, 2021

Thanks!

kleonc deleted the image-fill-rect branch

November 24, 2021 16:54

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

Labels

enhancement topic:core