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.

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

[combine texture modifier regression #14312

Closed
grorp opened this issue Jan 27, 2024 · 8 comments · Fixed by #14314
Closed

[combine texture modifier regression #14312

grorp opened this issue Jan 27, 2024 · 8 comments · Fixed by #14314
Labels
Bug Issues that were confirmed to be a bug @ Client rendering Regression Something that used to work no longer does.
Milestone
5.9.0

Comments

@grorp
Copy link
Member

grorp commented Jan 27, 2024 •
edited 

Minetest 5.9.0-dev-89f3502b56 (Linux)
Using Irrlicht 1.9.0mt14
Using LuaJIT 2.1.1692716794
Running on Linux/6.6.6-200.fc39.x86_64 x86_64
BUILD_TYPE=RelWithDebInfo
RUN_IN_PLACE=1
USE_CURL=1
USE_GETTEXT=1
USE_SOUND=1
STATIC_SHAREDIR="."
STATIC_LOCALEDIR="locale"

Steps to reproduce

  1. Start a MineClone 2 game
  2. Execute /giveme mcl_heads:creeper
  3. Place it

Result

Broken rendering
screenshot broken

generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-36,4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-36,4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-36,4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-36,4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-44,4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-44,4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-28,0=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-28,0=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-44,0=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-44,0=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-52,0=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-52,0=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-36,0=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-36,0=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-36,-4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-36,-4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-52,-4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-52,-4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-40,-4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-40,-4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-32,-4=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-32,-4=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-36,0=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-36,0=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-36,0=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-36,0=mcl_heads_creeper.png"
generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x16:-44,8=mcl_heads_creeper.png", cancelling.
generateImage(): Failed to generate "[combine:16x16:-44,8=mcl_heads_creeper.png"
...

Working rendering (with b7822e7 reverted)
screenshot working

Looks like it doesn't accept negative offsets anymore?
@grorp grorp added Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible @ Client rendering Regression Something that used to work no longer does. labels Jan 27, 2024
@Zughy Zughy added this to the 5.9.0 milestone Jan 27, 2024
@appgurueu
Copy link
Contributor

appgurueu commented Jan 27, 2024 •
edited

It looks like the code inappropriately used unsigned types when it should have used signed types; we were relying on undefined implementation-defined behavior, it seems (unsigned -> signed conversion). sfan introduced what (from our perspective) looked like a safe change:

if (x >= w0 || y >= h0)
    COMPLAIN_INVALID("X or Y offset");

but which fails if x / y are really negative s32s which have their sign bit (highest bit) set, and which are implicitly and undefinedly reconverted into s32 when converted into a v2s32 for blitting. I really hate C++'s implicit integer conversions sometimes...

This could have been caught with -Wsign-conversion, but we currently get too much spam with that.

The fix should be trivial: Change these to s32. Though I'm not sure we want to keep this check at all. Blitting out of bounds should probably just be a valid no-op.

@appgurueu appgurueu added Bug Issues that were confirmed to be a bug and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Jan 27, 2024
@appgurueu appgurueu mentioned this issue Jan 27, 2024
Merged
@sfan5
Copy link
Member

sfan5 commented Jan 27, 2024 •
edited

As far as I am concerned passing negative coordinates or otherwise out-of-bounds coordinates should be an error. 🤷
It sounds like MCL2 should have been using [sheet instead.

Blitting out of bounds should probably just be a valid no-op.

I don't think it's hard to imagine that "if it looks like an error silently do nothing" is often not very helpful.

@Sokomine
Copy link
Contributor

Probably related: Signs from signs_lib (guess it's the posters) scroll horribly in chat with red warning messages (this is probably from a single poster):

ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-4,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-4,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-36,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-36,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:0,-16=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:0,-16=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-40,-44=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-40,-44=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:4,-32=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:4,-32=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-4,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-4,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-36,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-36,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:0,-16=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:0,-16=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-40,-44=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-40,-44=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:4,-32=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:4,-32=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-4,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-4,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-36,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-36,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:0,-16=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:0,-16=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-40,-44=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-40,-44=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:4,-32=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:4,-32=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-4,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-4,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-36,-8=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-36,-8=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-12=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-12=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-16,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-16,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-44,-28=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-44,-28=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:0,-16=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:0,-16=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:-40,-44=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:-40,-44=(character.png)"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:16x32:4,-32=(character.png)", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:16x32:4,-32=(character.png)"

@grorp
Copy link
Member Author

grorp commented Jan 27, 2024

As far as I am concerned passing negative coordinates or otherwise out-of-bounds coordinates should be an error.

There's no reason for an error if part of the image is still in the visible area, which is possible with negative coordinates. Anyway, I think we have to support this for backwards compatibility.

@appgurueu
Copy link
Contributor

As far as I am concerned passing negative coordinates or otherwise out-of-bounds coordinates should be an error. 🤷

Passing negative coordinates should definitely not be an error. That's how you achieve (arbitrary) cropping of images.

It sounds like MCL2 should have been using [sheet instead.

I don't think they can. Not in general anyways. They basically want to crop the creeper texture to only keep the head. If the creeper texture conveniently happened to align to a 16x16 grid which could be interpreted as a tilesheet, this would work. But in all other cases, it doesn't. Looking at the texture modifiers, we see [combine:16x16:-36,4... for example. Note that neither -36 nor 4 are multiples of 16. So [sheet (alone) isn't sufficient here.

Blitting out of bounds should probably just be a valid no-op.

I don't think it's hard to imagine that "if it looks like an error silently do nothing" is often not very helpful.

We could consider a "special case" where blitting an image completely out of bounds (that is, zero overlap with the original image) is an error. I don't think this is a good idea, for multiple reasons:

  • Backwards compatibility.
  • Valid use cases (keep in mind that modders don't always know texture resolutions.)
  • It's completely well defined, and in my opinion good to have for consistency. Fewer special cases.
  • I don't think I've seen any other game engine make this an error. Blitting something off-clip-area is just a sensible no-op.

@Sokomine
Copy link
Contributor

And here for signs_lib_color and others as requested (only some example lines; multi-line errors combined back into one line):

ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:32x64:56,20=yl_npc_cape_default.png", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:32x64:56,20=yl_npc_cape_default.png"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:32x64:56,20=blank.png", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:32x64:56,20=blank.png"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:291x102:83,0=signs_lib_color_16px_0.png:211,0=signs_lib_color_16px_0.png:83,0=signs_lib_font_16px_42.png:94,0=signs_lib_font_16px_61.png:102,0=signs_lib_font_16px_72.png:108,0=signs_lib_font_16px_72.png:114,0=signs_lib_font_16px_61.png:122,0=signs_lib_font_16px_63.png:130,0=signs_lib_font_16px_75.png:139,0=signs_lib_font_16px_64.png:148,0=signs_lib_font_16px_61.png:156,0=signs_lib_font_16px_20.png:162,0=signs_lib_font_16px_50.png:172,0=signs_lib_font_16px_61.png:180,0=signs_lib_font_16px_72.png:186,0=signs_lib_font_16px_6b.png:195,0=signs_lib_font_16px_20.png:201,0=signs_lib_color_16px_n.png:329,0=signs_lib_color_16px_n.png:83,16=signs_lib_color_16px_n.png:211,16=signs_lib_color_16px_n.png", cancelling.

ERROR[Main]: generateImage(): Failed to generate "[combine:291x102:83,0=signs_lib_color_16px_0.png:211,0=signs_lib_color_16px_0.png:83,0=signs_lib_font_16px_42.png:94,0=signs_lib_font_16px_61.png:102,0=signs_lib_font_16px_72.png:108,0=signs_lib_font_16px_72.png:114,0=signs_lib_font_16px_61.png:122,0=signs_lib_font_16px_63.png:130,0=signs_lib_font_16px_75.png:139,0=signs_lib_font_16px_64.png:148,0=signs_lib_font_16px_61.png:156,0=signs_lib_font_16px_20.png:162,0=signs_lib_font_16px_50.png:172,0=signs_lib_font_16px_61.png:180,0=signs_lib_font_16px_72.png:186,0=signs_lib_font_16px_6b.png:195,0=signs_lib_font_16px_20.png:201,0=signs_lib_color_16px_n.png:329,0=signs_lib_color_16px_n.png:83,16=signs_lib_color_16px_n.png:211,16=signs_lib_color_16px_n.png"


ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:32x64:56,20=yl_npc_cape_haven_1.png", cancelling.
ERROR[Main]: generateImage(): Failed to generate "[combine:32x64:56,20=yl_npc_cape_haven_1.png"
ERROR[Main]: generateImagePart(): invalid X or Y offset for part_of_name="[combine:125x59:-4,-2=font_tinycurs_0021.png:1,-2=font_tinycurs_0020.png:10,-2=font_tinycurs_0022.png:17,-2=font_tinycurs_0020.png:26,-2=font_tinycurs_0023.png:36,-2=font_tinycurs_0020.png:45,-2=font_tinycurs_0024.png:59,-2=font_tinycurs_0020.png:68,-2=font_tinycurs_0025.png:82,-2=font_tinycurs_0020.png:91,-2=font_tinycurs_0026.png:103,-2=font_tinycurs_0020.png:112,-2=font_tinycurs_0027.png:115,-2=font_tinycurs_0020.png:124,-2=font_tinycurs_0028.png:-8,9=font_tinycurs_0020.png:0,9=font_tinycurs_0029.png:6,9=font_tinycurs_0020.png:15,9=font_tinycurs_002a.png:24,9=font_tinycurs_0020.png:33,9=font_tinycurs_002b.png:41,9=font_tinycurs_0020.png:50,9=font_tinycurs_002c.png:53,9=font_tinycurs_0020.png:62,9=font_tinycurs_002d.png:69,9=font_tinycurs_0020.png:78,9=font_tinycurs_002e.png:81,9=font_tinycurs_0020.png:90,9=font_tinycurs_002f.png:99,9=font_tinycurs_0020.png:108,9=font_tinycurs_0030.png:117,9=font_tinycurs_0020.png:-2,20=font_tinycurs_0032.png:7,20=font_tinycurs_0020.png:16,20=font_tinycurs_0033.png:25,20=font_tinycurs_0020.png:34,20=font_tinycurs_0034.png:43,20=font_tinycurs_0020.png:52,20=font_tinycurs_0035.png:62,20=font_tinycurs_0020.png:71,20=font_tinycurs_0036.png:81,20=font_tinycurs_0020.png:90,20=font_tinycurs_0037.png:99,20=font_tinycurs_0020.png:108,20=font_tinycurs_0038.png:118,20=font_tinycurs_0020.png:-4,31=font_tinycurs_0039.png:3,31=font_tinycurs_0020.png:12,31=font_tinycurs_003a.png:17,31=font_tinycurs_0020.png:26,31=font_tinycurs_003b.png:31,31=font_tinycurs_0020.png:40,31=font_tinycurs_003c.png:48,31=font_tinycurs_0020.png:57,31=font_tinycurs_003d.png:65,31=font_tinycurs_0020.png:74,31=font_tinycurs_003e.png:82,31=font_tinycurs_0020.png:91,31=font_tinycurs_003f.png:99,31=font_tinycurs_0020.png:108,31=font_tinycurs_0040.png:120,31=font_tinycurs_0020.png:6,42=font_tinycurs_0041.png:15,42=font_tinycurs_0020.png:24,42=font_tinycurs_0042.png:31,42=font_tinycurs_0020.png:40,42=font_tinycurs_0043.png:49,42=font_tinycurs_0020.png:58,42=font_tinycurs_0044.png:68,42=font_tinycurs_0020.png:77,42=font_tinycurs_0045.png:85,42=font_tinycurs_0020.png:94,42=font_tinycurs_0046.png:102,42=font_tinycurs_0020.png:111,42=font_tinycurs_0047.png", cancelling.

@appgurueu
Copy link
Contributor

appgurueu commented Jan 27, 2024 •
edited

Regarding Sokomine's comment:

  • [combine:125x59:-4,-2=font_tinycurs_0021.png - that's a negative offset there.
  • [combine:32x64:56,20=blank.png - looks like entirely out of bounds coordinates are used (whether it's a bug or not).

My PR should return both to the previous behavior (proper blitting in the first case, silent no-op in the latter case).

@appgurueu appgurueu changed the title MineClone 2 "mob head" broken by tile generation changes [combine texture modifier regression Jan 27, 2024
@sfan5
Copy link
Member

sfan5 commented Jan 27, 2024

Anyway, I think we have to support this for backwards compatibility.

Indeed.

Passing negative coordinates should definitely not be an error. That's how you achieve (arbitrary) cropping of images.

My mental model for [combine was that you get a big canvas and the ability the put smaller images on it.
Sure, in an edge case an image might go off the edge of the canvas but the norm is that it fits inside.

The fact that negative offsets safely and reliably do what they do and can be used to get arbitrary cropping surprised me and seems more like a happy accident to me.

Consider also that the code has no special casing for this and blindly processes 262144 pixels when you want to "crop" anything (no matter how small) from a tilesheet sized 512x512.
No wonder people complain about performance issues at times.

We could consider a "special case" where blitting an image completely out of bounds (that is, zero overlap with the original image) is an error.

I was thinking this should be made a warning at least.

  • Valid use cases (keep in mind that modders don't always know texture resolutions.)

As far as I can see all relevant parameters are in control of the mods here.

Blitting something off-clip-area is just a sensible no-op.

Only issue is that it's not currently a no-op (by work done, not end result).

@cx384 cx384 mentioned this issue Feb 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Issues that were confirmed to be a bug @ Client rendering Regression Something that used to work no longer does.
Projects
None yet
Milestone
5.9.0
Development

Successfully merging a pull request may close this issue.

Two tiny doc things, combine regression fix appgurueu/minetest
5 participants
@sfan5 @Sokomine @appgurueu @Zughy @grorp