Wrong texture offsets with ZDBSP-built extended nodes

[d-bind]

Texture offsets on the front side of a double-sided line's midtexture appear to be incorrect in maps built with ZDBSP. This only happens when the nodes are large enough that extended format gets forced - but not when extended nodes are forced from the command line on a smaller map.

I wasn't sure where to report this, but since it looks fine in ZDoom at least as far back as 2008, I'll assume that the nodes are working as intended, and are just being interpreted incorrectly by other ports (Woof, DSDA, DoomRetro).

Attached are two wads, the only difference is that one map has less out-of-bounds geometry than the other. Both were compiled using ZDBSP 1.19 with --extended command line switch.
When played in non-GZ ports, the texture closest to the starting point gets shifted by the amount of pixels equal to the line length (?). In (G)ZDoom it appears as it should (with left side anchored in place) - though it might be because GZ disregards existing nodes and builds its own.

The attached maps also exhibit tiling issues on non-power-of-two textures. To be clear, that is not the subject of this report. The primary issue is with the 128 texture getting an offset that shouldn't be there, and said offset depending on map complexity. The root cause could be the same as with tiling though.

[d-bind]

Note than when I said "large" I meant "numerous". The sectors and lines are regular in size.
I don't know what "extended" means in terms of data, but how can precision affect only the front side, and not the back side?

[fabiangreffrath]

I guess this has to do with the number of sidedefs exceeding the value of SHRT_MAX. 🤷

[fabiangreffrath]

I think I may have found the culprit. I inspected the map in Eureka and realized that one of the lines in question 33325 had front sidedef 33987 and back sidedef 33986. However, if I load the map in Woof, the line has sidedef 230 ob both sides:

(gdb) p lines[33325]
$1 = {
  v1 = 0x7fffc88f1b88,
  v2 = 0x7fffc88f1b98,
  dx = 4194304,
  dy = 0,
  flags = 276,
  special = 0,
  tag = 0,
  sidenum = {
    230,
    230
  },
  bbox = {
    163577856,
    163577856,
    266338304,
    270532608
  },
  slopetype = ST_HORIZONTAL,
  frontsector = 0x7fffc90dd828,
  backsector = 0x7fffc90dd828,
  validcount = 0,
  specialdata = 0x0,
  tranlump = -1,
  firsttag = -1,
  nexttag = 33326
}

Also, there are only 33834 sidedef in the map:

(gdb) p numsides
$2 = 33834

So, what happens in Eureka? See the following output:

Loading initial map : MAP01
Unpacked 31809 shared sidedefs --> 65684

So, it is "unpacking" shared sidedefs. What does this mean?

The function that returns this message is ChecksModule::sidedefsUnpack(). It iterates through all linedefs in the map and makes sure that each side points to an individual sidedef. It does so by creating copies of the sidedefs, which is why it ends up with 65684 sidedefs, whereas the map data itself only contains 33834. Apparently GZDoom (and DSDA-Doom with the OpenGL renderer, and probably any other port with an OpenGL renderer) does the same.

Apparently, the trick to "pack" sidedefs was applied by the node builder in order to keep the number of nodes within the range of numbers that can be represented by short types. There is also the following message right above the function in Eureka:

static const char *const unpack_confirm_message =
	"This map contains shared sidedefs.  It it recommended to unpack "
	"them, otherwise it may cause unexpected behavior during editing "
	"(such as random walls changing their texture).\n\n"
	"Unpack the sidedefs now?";

So, now the question remains if we want to do what any other software rendering port does and take the map data as it comes. Or if we want to reinterpret map data and make sure that each side of each line points to its own individual sidedef?

[d-bind]

So does this mean these sides effectively overflow, wrap around, and use some unrelated side's offsets in software renderers? Seems questionable to just leave in for port parity. I'd say if the ports support the format, then they should support the format.

Not sure I understood why it only seems to affect these midtextures, either - are non-sector-forming lines stored last or something? Or did I just happen to luck into it multiple times while making minimal repro?

[fabiangreffrath]

This fixes the issue - but of course breaks any other map. 😆

--- a/src/r_segs.c
+++ b/src/r_segs.c
@@ -780,7 +780,7 @@ void R_StoreWallRange(const int start, const int stop)
     {
       // [FG] fix long wall wobble
       rw_offset = (fixed_t)(((dx * dx1 + dy * dy1) / len) << 1);
-      rw_offset += sidedef->textureoffset + curline->offset;
+      rw_offset += sidedef->textureoffset; // + curline->offset;
 
       rw_centerangle = ANG90 + viewangle - rw_normalangle;
 

[fabiangreffrath]

So does this mean these sides effectively overflow, wrap around, and use some unrelated side's offsets in software renderers? Seems questionable to just leave in for port parity. I'd say if the ports support the format, then they should support the format.

The same sidedefs (with their textureoffset values) are applied to different lines (with their individual offset values) and this just cannot match all the time.

[fabiangreffrath]

Not sure I understood why it only seems to affect these midtextures, either - are non-sector-forming lines stored last or something? Or did I just happen to luck into it multiple times while making minimal repro?

It has nothing to do with the map format itself, that's properly supported. It has to do with the trick that the node builder has applied to the map data to make sure if doesn't have to store more than the supported number of individual sidedefs.