All Projects FSSCP  wxFRED

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000476FSSCPDirectXpublic2005-07-15 00:262006-11-01 04:29
ReporterGoober5000 Assigned Totaylor  
PriorityimmediateSeverityblockReproducibilityalways
Status closedResolutionunable to reproduce 
Summary0000476: CTD whenever a popup window appears
DescriptionWhen I run Freespace in D3D, whether in windowed mode or not, I get an Access Violation whenever a popup window starts to appear. (This can be seen with the new pilot tips popup, the mission listing popup, etc.)

The bug appears here in gr_d3d_save_screen() in grd3d.cpp:
[EDIT: Tabs don't seem to work here. It's the line beginning with >>>.]

if(D3D_32bit) {
    for(int j = 0; j < gr_screen.max_h; j++) {
        TmpC *src = (TmpC *) (((char *) src_rect.pBits) + (src_rect.Pitch * j));
        uint *dst = (uint *) (((char *) dst_rect.pBits) + (dst_rect.Pitch * j));
        
        for(int i = 0; i < gr_screen.max_w; i++) {
>>> dst[i] = 0;
            dst[i] |= (uint)(( (int) src[i].r / r_gun.scale ) << r_gun.shift);
            dst[i] |= (uint)(( (int) src[i].g / g_gun.scale ) << g_gun.shift);
            dst[i] |= (uint)(( (int) src[i].b / b_gun.scale ) << b_gun.shift);
        }
    }
} else {

where i == 736 and j == 767.

This is in 1024x768x32 mode. Oddly enough it only happens in debug builds, not in release builds.

This is on a fresh CVS checkout.
Steps To ReproduceStart Freespace. Create a new pilot. Watch the popup cause a CTD.
Additional InformationCall stack:
gr_d3d_save_screen() line 1650 + 12 bytes
popup_do(popup_info * 0x02061318 Popup_info, int 67175424) line 1024 + 8 bytes
popup(int 67175424, int 3) line 1216 + 14 bytes
player_tips_popup() line 1718 + 76 bytes
main_hall_do(float 0.250000) line 1503
game_do_state(int 1) line 7973 + 12 bytes
gameseq_process_events() line 657 + 21 bytes
WinMainSub(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00000000, char * 0x81d864d1, int 1) line 8638 + 5 bytes
WinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00000000, char * 0x81d864d1, int 1) line 8687 + 21 bytes
WinMainCRTStartup() line 198 + 54 bytes
KERNEL32! bff8b560()
KERNEL32! bff8b412()
KERNEL32! bff89dd5()
TagsNo tags attached.

Relationships

related to 0000521 closedtaylor A mouse graphics distortion when trying to quit fsscp 

Activities

Goober5000

2005-07-15 01:29

administrator   ~0002779

Rolling grd3d.cpp back to version 2.83 (just before taylor's changes on 6/19) fixes it.

taylor

2005-07-15 01:57

administrator   ~0002781

I never had that problem when I initially wrote the code and tested it pretty well before adding to CVS. My changes were to only affect src_rect, not dst_rect so I'm not really sure why it's having the trouble that it is.

Is this just happening with current CVS or does it happen with something like my 20050624 build too? This happens in 16-bit color mode as well right?

Goober5000

2005-07-15 02:25

administrator   ~0002784

Okay, test results:

fs2_open_T-20050624-dbg.exe crashes in the same way under the same conditions: D3D 1024x768x32 when a popup is set to appear.

Neither fs2_open_T-20050624-dbg.exe nor my CVS build crashes under D3D 1024x768x16, curiously enough.

Just to make sure, I tested it with 640x480. Both crash in 32-bit mode but neither crashes in 16-bit mode.

taylor

2005-07-15 02:37

administrator   ~0002785

Hmm, well that throws my Goober-blame-shift out the window. ;)

I'll test it first thing in the morning (or tonight if I can't get to sleep). Don't roll it back yet but if I don't fix it by the weekend then go ahead. This fix isn't something worth leaving in it it's causing crashes. Just make sure that the screen shot code is working properly in both color modes too since the changes to gr_d3d_save_screen() are based on that (gr_d3d_print_screen()).

Goober5000

2005-07-15 03:04

administrator   ~0002788

Last edited: 2005-07-15 03:07

 Fixed it! :)

I figured it out by looking at the CVS diff. On a hunch, I changed your line
>>> mode.Width, mode.Height, D3DFMT_A8R8G8B8, &front_buffer_a8r8g8b8))) {
back to
>>> gr_screen.max_w, gr_screen.max_h, D3DFMT_A8R8G8B8, &front_buffer_a8r8g8b8))) {
and it started working again.

After looking at the file for a few minutes, I think I know why. The problem area was in the middle of a loop that used gr_screen.max_h and gr_screen.max_w, but you hadn't changed these like you changed the ones above. The bug was probably a simple array-out-of-bounds or something. I'm guessing they should be the same thing in both places - either both using gr_screen or both using mode.

edited on: 07-14-05 23:07

taylor

2005-07-15 03:53

administrator   ~0002789

That's kinda what I was thinking too. Just didn't have a way to test it yet. The problem though is that it probably breaks it working in a window again. Well, you're change certainly does but what to do about it is the question. The front buffer has the be the full screen res since the front buffer is that size. If you run at 1280x1024 desktop res and the game is 640x480 in a window then the front buffer is 1280x1024 but we can only use a specific 640x480 region of it. Gr_saved_surface on the other hand has to be gr_screen.max_w by gr_screen.max_h in size, not what "mode" is set to.

I think what we need to do is change Gr_saved_surface to work the way OGL does it. Basically, grab the buffer, copy out the data we need into a vm_malloc()'d area, and bm_create() out of it. It will end up in bmpman and then we can do other things with it if wanted, without it being API specific. What do you think?

Goober5000

2005-07-15 05:54

administrator   ~0002790

Last edited: 2005-07-15 05:54

 "That's kinda what I was thinking too. Just didn't have a way to test it yet. The problem though is that it probably breaks it working in a window again. Well, you're change certainly does but what to do about it is the question. The front buffer has the be the full screen res since the front buffer is that size. If you run at 1280x1024 desktop res and the game is 640x480 in a window then the front buffer is 1280x1024 but we can only use a specific 640x480 region of it. Gr_saved_surface on the other hand has to be gr_screen.max_w by gr_screen.max_h in size, not what "mode" is set to."

Um... I just barely followed that. :)

But keep in mind that it works perfectly in Release builds... it's just Debug builds that don't work for some reason. (And that's probably why nobody reported any problems... only coders really use debug builds.) If we can figure out why then we won't have to change it (much).

"I think what we need to do is change Gr_saved_surface to work the way OGL does it. Basically, grab the buffer, copy out the data we need into a vm_malloc()'d area, and bm_create() out of it. It will end up in bmpman and then we can do other things with it if wanted, without it being API specific. What do you think?"

That sounds good I suppose - non-API specific is nice, and I suppose speed and memory aren't critical in this situation. What other things would we want to do with the screenshot?

Oh... and on a similar (maybe) note, is this at all related to the situation where if you're playing a mission and press ESC to bring up the "abort mission" dialog the surrounding screen turns black? In retail you could still see the area around the popup - messages, HUD, and such. It would be nice to have that fixed.

edited on: 07-15-05 01:54

Goober5000

2005-07-15 06:28

administrator   ~0002793

Hm.

I'm not sure what exactly might break with my code, but I just took a screenshot running my build and it seemed to work. It dumped a screenshot to my Freespace 2 directory that was just the area of the window and none of the surrounding desktop.

I tried it in fullscreen mode and it worked there too.

Perhaps "get the full mode size which for windowed mode is the entire screen and not just the window" is incorrect, and it *is* in fact just the window?

taylor

2005-07-15 06:40

administrator   ~0002794

Last edited: 2005-07-15 09:38

 "Um... I just barely followed that. :)"

Hey, I had to learn about that crap just to get it to work. I HATE DIRECTX!!! ... That pretty much sums up the experience. :)

So basically what I said, when running in a window:
1) the front buffer will be the size of the entire desktop, not the actual window
2) have to figure out the position of the window and create a rect which is just the part that we want, lock it there
3) the locked portion would be the size of gr_screen.max_w, gr_screen.max_h
4) the image we need on the back buffer has to be gr_screen.max_w by gr_screen.max_h in size, regardless of size of front buffer

Still a little hard to follow, but that's Microsoft for you. Going over the code though I actually still don't see a problem. "dst_rect" is gr_screen.max_w/max_h in size and "src_rect" is gr_screen.max_w/max_h in size. There should be no bounds problem. A simple bounds problem should have happened in 16-bit mode too unless there is something else wrong with the code and my change only exposed it. But I think you realize that already since it only happens in debug builds.

"That sounds good I suppose - non-API specific is nice, and I suppose speed and memory aren't critical in this situation. What other things would we want to do with the screenshot?"

Well it should actually be faster, not slower. Not sure about memory since it is going to end up being in system and API memory. It's not like this is used much though so the memory impact would be negligible. No clue what else we could do with it but we can just call gr_save_screen() and get a bitmap id back. Once we've got the id we can do any normal texture stuff that we want with the image. Doesn't appear to be all the useful to me but maybe WMC could get something out of it in the future.

Either way the changes are about ready should make a bit more sense than the crap that's there now. Works perfectly in windowed mode but fullscreen flickers for some reason. When I've got that fixed I'll get you the changes to test, just in case.

"Oh... and on a similar (maybe) note, is this at all related to the situation where if you're playing a mission and press ESC to bring up the "abort mission" dialog the surrounding screen turns black? In retail you could still see the area around the popup - messages, HUD, and such. It would be nice to have that fixed."

Yep, this is the same code and what I was trying to fix. I think it was really only a problem for windowed mode. I thought it worked ok fullscreen but I don't use D3D very much so maybe not.

EDIT: "Perhaps "get the full mode size which for windowed mode is the entire screen and not just the window" is incorrect, and it *is* in fact just the window?"

Nope, not according to MSDN or what I tested. There is extra code there which is probably covering the problem but the change you listed isn't really right. It would work fine in fullscreen mode but the front buffer is going to be what it is.

EDIT2: Oh and since you may not have realized it, gr_d3d_save_screen() isn't the same code that creates the screenshot. That's gr_d3d_print_screen(). gr_d3d_save_screen() is only the pop-unders which are the background when a popup window appears. It doesn't get used anywhere else.
edited on: 07-15-05 02:44

edited on: 07-15-05 05:38

Goober5000

2005-07-15 21:08

administrator   ~0002795

Yeah, I hate Microsoft programming style too. :)

"So basically what I said, when running in a window:
1) the front buffer will be the size of the entire desktop, not the actual window
2) have to figure out the position of the window and create a rect which is just the part that we want, lock it there
3) the locked portion would be the size of gr_screen.max_w, gr_screen.max_h
4) the image we need on the back buffer has to be gr_screen.max_w by gr_screen.max_h in size, regardless of size of front buffer"

Yeah, that's what I figured. That doesn't explain either the error or the fact that my code change seemed to work. Perhaps the MSDN documentation is wrong? :p

Well if you've gotten the new code mostly working then well done. :) I'm looking forward to seeing it. If it's at all better than the Microsoft gobbledegook, and it works, then I'll be happy. :)

taylor

2005-08-11 11:34

administrator   ~0002982

Have you had a chance to look at what I sent you Goober? I still don't know why it only worked perfectly when in a window unless there is something screwy with the fullscreen setup that I just overlooked.

Goober5000

2005-08-11 18:13

administrator   ~0002983

I haven't, sorry. I've been working on my thesis recently and haven't had much time to do much else.

I should have more time in two weeks or so.

phreak

2006-05-02 17:14

developer   ~0005453

another round of bumping old bugs on the way!

taylor

2006-05-08 04:11

administrator   ~0005479

* BUMP *

Still haven't seen anyone else report this and since most people use OpenGL now anyway, any point in not closing this? I'm not sure if that new code was ever tested to work but given the number of problems in D3D right now it doesn't make much sense to spend time figuring out this particular issue when the other stuff won't get fixed.

Goober5000

2006-05-08 12:48

administrator   ~0005486

I think it kept crashing for a while and then started to work. I'm not sure what the deal is currently. But I guess we can close this.

taylor

2006-05-08 15:58

administrator   ~0005487

Ok. Closing until it's an issue again.

Issue History

Date Modified Username Field Change
2005-07-15 00:26 Goober5000 New Issue
2005-07-15 00:27 Goober5000 Description Updated
2005-07-15 00:27 Goober5000 Description Updated
2005-07-15 00:28 Goober5000 Description Updated
2005-07-15 01:29 Goober5000 Note Added: 0002779
2005-07-15 01:35 Goober5000 Assigned To => taylor
2005-07-15 01:35 Goober5000 Status new => assigned
2005-07-15 01:57 taylor Note Added: 0002781
2005-07-15 02:25 Goober5000 Note Added: 0002784
2005-07-15 02:37 taylor Note Added: 0002785
2005-07-15 03:04 Goober5000 Note Added: 0002788
2005-07-15 03:05 Goober5000 Note Edited: 0002788
2005-07-15 03:05 Goober5000 Note Edited: 0002788
2005-07-15 03:07 Goober5000 Note Edited: 0002788
2005-07-15 03:07 Goober5000 Note Edited: 0002788
2005-07-15 03:53 taylor Note Added: 0002789
2005-07-15 05:54 Goober5000 Note Added: 0002790
2005-07-15 05:54 Goober5000 Note Edited: 0002790
2005-07-15 06:28 Goober5000 Note Added: 0002793
2005-07-15 06:40 taylor Note Added: 0002794
2005-07-15 06:44 taylor Note Edited: 0002794
2005-07-15 09:38 taylor Note Edited: 0002794
2005-07-15 21:08 Goober5000 Note Added: 0002795
2005-08-11 11:34 taylor Note Added: 0002982
2005-08-11 18:13 Goober5000 Note Added: 0002983
2006-05-02 17:14 phreak Note Added: 0005453
2006-05-08 04:11 taylor Note Added: 0005479
2006-05-08 12:48 Goober5000 Note Added: 0005486
2006-05-08 15:58 taylor Status assigned => closed
2006-05-08 15:58 taylor Note Added: 0005487
2006-11-01 04:29 taylor Resolution open => unable to reproduce
2006-11-01 04:34 taylor Relationship added related to 0000521