Thursday, November 12, 2009

DialogBox + Accelerators

Can't be done. Sorry. You have to write the modal loop yourself.

With the help of The Old New Thing (Yes, its not thread safe. Who even does UI from multiple threads anyway), this function does a standard modal dialog, except it dispatches F1 as IDHELP.

Big caveat! You can't call EndDialog() in your dialog proc - in this, you have to replace EndDialog() with s_ModalDialogResult = . You could also write a function to emulate EndDialog pretty trivially (along with making it thread safe)

static S32 s_ModalDialogResult = -1;
S32 DialogBoxWithHelp(LPCSTR i_Template, LPARAM i_Param, HWND i_Owner, DLGPROC i_Proc)
{
MSG msg;
s_ModalDialogResult = -1;

HWND hDialog = CreateDialogParam(hInstance, i_Template, i_Owner, i_Proc, i_Param);
if (hDialog == 0) return -1;

ShowWindow(hDialog, SW_SHOW);
EnableWindow(i_Owner, FALSE);

ACCEL helper[] =
{
{ FVIRTKEY, VK_F1, IDHELP }
};
HACCEL hAccel = CreateAcceleratorTable(helper, 1);

while (s_ModalDialogResult == -1)
{
S32 msgresult = GetMessage(&msg, 0, 0, 0);
if (msgresult > 0)
{
if (TranslateAccelerator(hDialog, hAccel, &msg) == 0 &&
IsDialogMessage(hDialog, &msg) == 0)
{
TranslateMessage(&msg);
DispatchMessage(&msg);
}
}
else
{
// We received a WM_QUIT message; bail out!
// Re-post the message that we retrieved
if (msgresult == 0) PostQuitMessage(msg.wParam);

// otherwise it was an error, bail.
break;
}
}

DestroyAcceleratorTable(hAccel);
EnableWindow(i_Owner, TRUE);
DestroyWindow(hDialog);

return s_ModalDialogResult;
}

References:
http://blogs.msdn.com/oldnewthing/archive/2005/02/18/376080.aspx
http://blogs.msdn.com/oldnewthing/archive/2005/02/24/379635.aspx
http://blogs.msdn.com/oldnewthing/archive/2005/02/22/378018.aspx

Monday, November 9, 2009

Generic Output Functions and Inlining

Today at work I encountered the following warning in release mode:

warning C4789: destination of memory copy is too small

The errant line was a standard function call - nothing crazy going on. For the sake of argument the function did something of the form:

GetValueByName("name of value", &OutputValue);

The function is declared GetValueByName(const char*, void*), and in the function it looked something like this:

ValueDescriptor Desc = FindValueDescriptor(PassedInNameOfValue);
return GetValueByDesc(Desc, PtrToOutputValue);

GetValueByDesc() worked somewhat like:

switch (Descriptor.Value)
case SomeFloatProperty: *(float*)PtrToOutputValue = MyFloatProperty;
case SomeVectorProperty: *(vector*)PtrToOutputValue = MyVectorProperty;
..etc..

This had been working for months (years actually) without error. And now when we compile in release, we get the aforementioned warning.

The compiler, as it happens, decided on this one callsite to inline *both* levels, and drop the switch statement in to the caller. As a result, in the code generation, there was a case that could write 12 bytes (via the vector dereference) to the output pointer, which triggered the warning.

Monday, October 26, 2009

Opening Explorer And Selecting A File

Can be done one of two ways.

ShellExecute(hwnd, 0, "explorer.exe", "/select,pathandfilename", NULL, SW_SHOWNORMAL);

Or:

wchar_t wpath[MAX_PATH];
MultiByteToWideChar(CP_UTF8, 0, path, -1, wpath, MAX_PATH);
ITEMIDLIST* hIdList = SHSimpleIDListFromPath(wpath);
SHOpenFolderAndSelectItems(hIdList, 0, 0, 0);
ILFree(hIdList);

Not sure as I'm a fan of the newer shell APIs.

Saturday, October 10, 2009

Sign Extention For Fun And Profit

An unexpected bug may come from code that looks like the following:

S32 My32BitInt = somenumber;
U32 Other32BitInt = anothernumber;
U64 MyLargeNumber = (U64)Other32BitInt + (U64)My32BitInt;

Spot the bug?

The code might not execute as expected - I don't know if this is spec or not, but the cast to U64 might not be a zero extend as you might expect - as the target is unsigned. In the above instance, we encountered a bug where the compiler was using cdq (sign extend), which when the value happens to be large enough, ends up adding a very large number.

To ensure zero extend, cast to unsigned of the same bit size beforehand:
U64 myval = (U64)(U32)My32BitInt;

Wednesday, October 7, 2009

Clamping isn't enough

Sometimes you might think that its enough to clamp an uninitialized variable to a known range, for instance [0, 1]. You might then think that you don't need no stinkin initializer, I got this handled.

I'm here to tell you that this is not sufficient.

Turns out floating point values have Special Places, where you can't ever touch. For instance, QNAN. Any operation on a QNAN is a QNAN, so these tend to migrate, and what's more, is since the number of possible 4 byte values that will give you a QNAN is fairly small, this bug will appear quite sporadically.

In my case, it would turn the entire game world black (or white, depending), as the value in question was the desaturation lerp parameter. Nominally clamped to [0, 1], when passed as a QNAN, it makes it all the way to the pixel shader, to turn the color of anything affected by the material to QNAN. Sweet action.

Wednesday, September 30, 2009

TreeViews and Check Boxes

Important side note when trying to insert checked items in to a newly created Win32 TreeView.

From the docs:

If you want to use this style, you must set the TVS_CHECKBOXES style with SetWindowLong after you create the treeview control, and before you populate the tree. Otherwise, the checkboxes might appear unchecked, depending on timing issues.


Good to know. Otherwise you might spend all morning trying to figure out why your inserted items don't appear checked. Also, I saw some confusion on the internet about how to insert a checked item. Witness:


TV_INSERTSTRUCT t = {0};
t.hParent = 0;
t.hInsertAfter = TVI_SORT;
t.item.mask = TVIF_TEXT | TVIF_PARAM | TVIF_STATE;
t.item.pszText = "text";
t.item.lParam = 1;
t.item.stateMask = ~0U; // set all state bits
t.item.state = INDEXTOSTATEIMAGEMASK(2); // 2 = checked
TreeView_InsertItem(TreeHwnd, &t);



Easy Peasy.

Another note from the docs:


Once a tree-view control is created with this style, the style cannot be removed. Instead, you must destroy the control and create a new one in its place. Destroying the tree-view control does not destroy the check box state image list. You must destroy it explicitly. Get the handle to the state image list by sending the tree-view control a TVM_GETIMAGELIST message. Then destroy the image list with ImageList_Destroy.



I did this via subclassing my TreeView and catching WM_DESTROY before forwarding it on to the default window procedure:


// Destroy our image list.
HIMAGELIST hCheckImages = TreeView_GetImageList(hwnd, TVSIL_STATE);
ImageList_Destroy(hCheckImages);

Friday, February 27, 2009

The Beginning Of The End

For God knows how long I've wanted a place to record interesting bugs/thoughts I've come across while coding. Well, here goes nothing. Theoretically this should end up as a pretty interesting list, but as goes the great quote, "In theory, theory matches practice, but in practice, it doesn't."