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

Portals2 visual #336

Closed
wants to merge 22 commits into from

Conversation

@jewalky
Copy link
Contributor

commented Jun 7, 2015

The latest VISUAL portal code. Means fully working line special 155. Line special 156 is available, but as of now behaves exactly like 155.

For other features, There's now a CVar r_portal_recursions that controls both mirror and portal recursions dynamically (only for horizontal portals for now, doesn't include plane portals), and r_highlight_portals CVar that helps debugging portal-related stuff on a level.
Plus, on top of that, this code adds advanced clipping for both mirrors and wall portals in software mode, making it possible to put mirrors on arbitrary places without having to put empty space behind them.

Name="Portals"
>
<File
RelativePath=".\src\portal.cpp"

This comment has been minimized.

Copy link
@edward-san

edward-san Jun 7, 2015

Contributor

This implies that there's also a new portal.cpp file, doesn't it? There's no new file in this pull request. Also, if this is needed, you need to update src/CMakeLists.txt as well.

This comment has been minimized.

Copy link
@jewalky

jewalky Jun 7, 2015

Author Contributor

Got somehow lost during the merge with master. Put it back. &updated the lists as well.

>
</File>
<File
RelativePath=".\src\portal.h"

This comment has been minimized.

Copy link
@edward-san

edward-san Jun 7, 2015

Contributor

Where's the portal.h file?

This comment has been minimized.

Copy link
@jewalky

jewalky Jun 7, 2015

Author Contributor

The same place where portal.cpp was. Fixed, too, in 31cad85.

@@ -236,4 +236,7 @@ DEFINE_SPECIAL(Ceiling_LowerToLowest, 253, 2, 2, 2)
DEFINE_SPECIAL(Ceiling_LowerToFloor, 254, 2, 2, 2)
DEFINE_SPECIAL(Ceiling_CrushRaiseAndStaySilA, 255, 4, 5, 5)

DEFINE_SPECIAL(Line_SetVisualPortal, 155, -1, -1, 3)
DEFINE_SPECIAL(Line_SetPortal, 156, -1, -1, 3)

This comment has been minimized.

Copy link
@Blzut3

Blzut3 Oct 4, 2015

Collaborator

Please keep these in numerical order. What do you mean by the comment "// vavoom? interferes with Line_SetVisualPortal"?

This comment has been minimized.

Copy link
@jewalky

jewalky Oct 9, 2015

Author Contributor

Nothing anymore. This is a relic of ancient past when Line_SetVisualPortal was 157 instead of 155. It's long fixed and this comment means nothing.

// [ZZ] check depth. fill portal with black if it's exceeding the visual recursion limit, and continue like nothing happened.
if (depth > r_portal_recursions)
{
BYTE color = (BYTE)BestColor((DWORD *)GPalette.BaseColors, 0, 0, 0, 0, 255);

This comment has been minimized.

Copy link
@Blzut3

Blzut3 Oct 4, 2015

Collaborator

How is the code that follows different from R_HighlightPortal? More specifically could r_highlight_portals just change the color variable or am I missing some detail?

This comment has been minimized.

Copy link
@jewalky

jewalky Oct 7, 2015

Author Contributor

The code in R_HighlightPortal draws red lines around the drawseg. The code in R_EnterPortal fills the whole drawseg area with black color in columns.

This comment has been minimized.

Copy link
@Blzut3

Blzut3 Oct 7, 2015

Collaborator

Ah, I see. Might want to describe that in a comment for that function then since it's not immediately obvious.

This comment has been minimized.

Copy link
@jewalky

jewalky Oct 8, 2015

Author Contributor

What R_HighlightPortal does is obvious once you see it in the map. That is if you don't see that it calls DrawLine.
And what the R_EnterPortal does is already described: "...fill portal with black if it's exceeding the visual recursion limit..." — from the very first comment in R_EnterPortal.

@@ -2495,6 +2564,8 @@ void R_DrawParticle (vissprite_t *vis)
fg = fg2rgb[color];
}

/*

This comment has been minimized.

Copy link
@Blzut3

Blzut3 Oct 4, 2015

Collaborator

I think we can let git keep track of the old version.

return abs(d2x);
}

void P_NormalizeVXVY(fixed_t& vx, fixed_t& vy)

This comment has been minimized.

Copy link
@Blzut3

Blzut3 Oct 4, 2015

Collaborator

Looks like remains of the non-visual part of the portal code? That's fine, but perhaps we need a warning here that this is potentially introducing floating point into the playsim code?

This comment has been minimized.

Copy link
@jewalky

jewalky Oct 7, 2015

Author Contributor

There are remains of the non-visual part, but this code isn't currently executed from anywhere, well, at least it shouldn't be.

bool portal;
bool portal_mirror;
bool portal_passive;
line_t *portal_dst;

This comment has been minimized.

Copy link
@Blzut3

Blzut3 Oct 4, 2015

Collaborator

A bit nit-picky, but since this ultimately affects the software renderer I thought I would mention it anyway: it might be best to reorder these so the portal_dst pointer is first. In general for best structure packing/alignment/cache friendliness you want the largest elements first.

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Oct 4, 2015

Crash in R_ClipSpriteColumnWithPortals with Urban Brawl (possibly with floor/ceiling portals since the wall portal test map works flawlessly).

@jewalky

This comment has been minimized.

Copy link
Contributor Author

commented Oct 7, 2015

The only way it could crash there is if seg->curline was null. Is that even possible?
I'm currently not in a place where I could check that. Will check ASAP.

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Oct 25, 2015

Another action doom portal crash. Warp to map04, then warp to map06. (Going directly to map06 works fine.) Appears to be some kind of dangling pointer.

Sorry it took so long to check the new changes. I either missed the notification or github didn't send me one regarding the commits so just noticed them a few days ago.

@jewalky

This comment has been minimized.

Copy link
Contributor Author

commented Nov 3, 2015

Could not reproduce this with the current version. Are you sure you built the right one?

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Nov 4, 2015

Yes. Changeset e669e3c. Not sure if I updated my Urban Brawl IWAD or not, but the md5sum of mine is c106a4e0a96f299954b073d5f97240be. It's possible it's only manifesting on Linux in which case I can try to figure out what's going on.

(gdb) run -iwad action2.wad +map map04
Starting program: /home/blzut3/Code/ZDoom/build/zdoom -iwad action2.wad +map map04
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
ZDoom 2.8pre-1653-ge669e3c - 2015-10-09 16:03:59 +0300 - SDL version
Compiled on Nov  3 2015

M_LoadDefaults: Load system defaults.
Gameinfo scan took 0 ms
W_Init: Init WADfiles.
 adding /home/blzut3/Code/ZDoom/build/zdoom.pk3, 582 lumps
 adding /usr/games/Doom-Data/iwads/action2.wad, 3240 lumps
I_Init: Setting up machine state.
CPU Vendor ID: GenuineIntel
  Name: Intel(R) Core(TM) i5 CPU 760 @ 2.80GHz
  Family 6, Model 30, Stepping 5
  Features: MMX SSE SSE2 SSE3 SSSE3 SSE4.1 SSE4.2
I_InitSound: Initializing FMOD
FMOD Sound System, copyright � Firelight Technologies Pty, Ltd., 1994-2009.
Loaded FMOD version 4.26.36
[New Thread 0x7fffe7f3e700 (LWP 3958)]
[Thread 0x7fffe7f3e700 (LWP 3958) exited]
[New Thread 0x7fffe7f3e700 (LWP 3959)]
[New Thread 0x7fffe773d700 (LWP 3960)]
[New Thread 0x7fffe6f3c700 (LWP 3961)]
V_Init: allocate screen.
S_Init: Setting up sound.
ST_Init: Init startup screen.
Checking cmd-line parameters...
S_InitData: Load sound definitions.
G_ParseMapInfo: Load map definitions.
Texman.Init: Init texture manager.
ParseTeamInfo: Load team definitions.
LoadActors: Load actor definitions.
R_Init: Init UrbanBrawl refresh subsystem.
DecalLibrary: Load decals.
Adding dehacked patch action2.wad:DEHACKED
Patch installed
M_Init: Init menus.
[New Thread 0x7fffe673b700 (LWP 3962)]
[Thread 0x7fffe673b700 (LWP 3962) exited]
[New Thread 0x7fffe673b700 (LWP 3963)]
P_Init: Init Playloop state.
ParseSBarInfo: Loading default status bar definition.
ParseSBarInfo: Loading custom status bar definition.
D_CheckNetGame: Checking network game status.
player 1 of 1 (1 nodes)
[Thread 0x7fffe673b700 (LWP 3963) exited]
[New Thread 0x7fffe673b700 (LWP 3964)]
Using video driver x11
Resolution: 1280 x 720


map04 - Action Subway: Follow the Bad Dudes

]map map06


map06 - Phylex: Rising to the Challenge


Program received signal SIGSEGV, Segmentation fault.
0x000000000071c142 in R_ClipSpriteColumnWithPortals (x=145883136, y=-15859712, spr=0x3b6d0d0) at ../src/r_things.cpp:338
338                     line_t* line = seg->curline->linedef;
(gdb) print seg
$1 = (drawseg_t *) 0x4c82310
(gdb) print *seg
$2 = {curline = 0xcececececececece, light = -825307442, lightstep = -842150194, iscale = -842150451, 
  iscalestep = -842150451, x1 = 0, x2 = 434, sx1 = -12851, sx2 = -12594, sz1 = 0, sz2 = 0, siz1 = 2147483647, 
  siz2 = 2147483647, cx = -825307442, cy = -825307442, cdx = -825307442, cdy = -825307442, yrepeat = -137441586, 
  silhouette = 3 '\003', bFogBoundary = 0 '\000', bFakeBoundary = 247 '\367', shade = 0, sprtopclip = 85858, 
  sprbottomclip = 85423, maskedtexturecol = -1, swall = -1, fake = 0, bkup = 0, tmapvals = {UoverZorg = 0, 
    UoverZstep = -1.00586491e+34, InvZorg = -1.00588019e+34, InvZstep = -1.00588019e+34}, CurrentPortalUniq = 0}
(gdb) print seg->curline
$3 = (seg_t *) 0xcececececececece
(gdb) print seg->curline->linedef
Cannot access memory at address 0xcececececececee6
(gdb) bt
#0  0x000000000071c142 in R_ClipSpriteColumnWithPortals (x=145883136, y=-15859712, spr=0x3b6d0d0)
    at ../src/r_things.cpp:338
#1  0x000000000071c4be in R_DrawVisSprite (vis=0x3b6d0d0) at ../src/r_things.cpp:431
#2  0x0000000000721476 in R_DrawSprite (spr=0x3b6d0d0) at ../src/r_things.cpp:2212
#3  0x00000000007216d4 in R_DrawMaskedSingle (renew=false) at ../src/r_things.cpp:2271
#4  0x00000000007217e9 in R_DrawMasked () at ../src/r_things.cpp:2308
#5  0x000000000070d1af in R_DrawSkyBoxes () at ../src/r_plane.cpp:1337
#6  0x00000000007092e9 in R_RenderActorView (actor=0x3b3fb30, dontmaplines=false) at ../src/r_main.cpp:929
#7  0x00000000006f7e2e in FSoftwareRenderer::RenderView (this=0x10da7f0, player=0xd0ede0 <players>)
    at ../src/r_swrenderer.cpp:111
#8  0x00000000005a8fbe in D_Display () at ../src/d_main.cpp:762
#9  0x00000000005a9b1e in D_DoomLoop () at ../src/d_main.cpp:1006
#10 0x00000000005ad8fd in D_DoomMain () at ../src/d_main.cpp:2612
#11 0x0000000000567edd in main (argc=5, argv=0x7fffffffde18) at ../src/posix/sdl/i_main.cpp:317
…r skyboxes, which interfered with portal sprite clipping routine randomly.
@jewalky

This comment has been minimized.

Copy link
Contributor Author

commented Nov 11, 2015

It's probably fixed now. At least my reliable way to reproduce it (go to map02, die in a hole near start, go to map03) doesn't crash anymore.

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Nov 12, 2015

Yeah, that fixes the crashes. Now there's a huge performance regression. Go to urban brawl map07 and open the elevator door. For me that hallway is perfectly smooth (35+ fps) with the latest master, with the new portal code I get about a half a frame per second. The level slots after map07 may also have some performance issues, but I didn't directly compare them since they're no where near as bad.

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Jan 10, 2016

Gez noticed your new commits, so I tried them out. The MAP07 performance seems to be good now. MAP08 still has a performance regression at the river, but it's still playable. There also seems to be a release build only (not even relwithdebinfo) crash at startup on MAP01. Being that it's release mode only I'm guessing I'll need to debug and fix myself, but thought I would let you know about it anyway.

@Gaerzi

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2016

(I'm not jewalky.)

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2016

Kind of a strange regression since some mirrors work, but the mirrors in the bathrooms near the start of Urban Brawl MAP04 and MAP06 are broken.

@Blzut3

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2016

Nevermind, fixed it myself. Missing call to P_CheckPortal in the hexen format linedef loader. Moved P_CheckPortal call to P_FinishLoadingLineDef unless I'm missing something it works fine there and is called by Doom, Hexen, and UDMF loaders.

There is now a portal2_visual branch in the upstream repo.

@coelckers

This comment has been minimized.

Copy link
Collaborator

commented Jan 19, 2016

One semi-major problem: Upper and lower textures on portal lines are not drawn. If we want to support linked-line portals Eternity style we will need this.

@Blzut3 Blzut3 closed this Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.