Thread: Problem trying to retrieve data from combo box! (strange issue)

John your insight has led to a proper solution to the problem. I disabled the WM_ACTIVE message block and and then duplicated the WM_INITDIALOG code as I think I mentioned above.

Then by trail and error I just turned on and off a variety of functions in that code until I could get it to the minimum needed to run correctly. I added a return statement to the WM_INITDIALOG block too so this code only runs once. I think before it just dropped through the switch statement on init and into the WM_ACTIVE block because I removed the return.

Anyways....to cut a long story short it ran properly when the function setResolution was run twice. I'll post the original below:

Code:

void setResolution(HWND hDlg)
{   
    D3DObj.mComboResData.clear();
    D3DClass::resData _resData;
    int j = 0;  

    for (int i = 0; i < D3DObj.mResolutionPair.size(); i += 4)
    {
        _resData.w = D3DObj.mResolutionPair[i];
        _resData.h = D3DObj.mResolutionPair[i + 1];
        _resData.Hz = (double)(D3DObj.mResolutionPair[i + 2]) / D3DObj.mResolutionPair[i + 3];

        std::wstring _resolution = std::to_wstring(_resData.w) +
            L" x " + std::to_wstring(_resData.h);

        const WCHAR* resolution = _resolution.c_str();
        D3DObj.mComboResData.push_back(_resData);

        WPARAM nI = (WPARAM)SendMessage(GetDlgItem(hDlg, IDC_COMBO1), CB_ADDSTRING, (WPARAM)0, (LPARAM)resolution);
        SendMessage(GetDlgItem(hDlg, IDC_COMBO1), CB_SETITEMDATA, (WPARAM)nI, (LPARAM) & (D3DObj.mComboResData[j])); // change nI variable to 0 and it works (for first entry)
        j++;       

        #ifdef SKIP  // change to _DEBUG to display
            std::wstring _string = std::to_wstring(_resData.w) + L" "
                + std::to_wstring(_resData.h) + L"\n"
                + std::to_wstring(_resData.Hz) + L"\n"
                + std::to_wstring(nI) + L"\n\n"; // may as well print nI, too
            OutputDebugString(_string.c_str());
        #endif
    }

    return;
}

I wrote a duplicate of it called pingCombo which I used to replace the first call to setRefresh. Then I cannibalised everything in it down to the bare minimum to get it to work. I even noticed if I messed with the iterator size in the for loop it would affect whether or not it worked. Confusing but I knew I was still getting close.

Well....turns out I needed to completely fill out the mComboResData vector before running the for loop which uses the indexing for the combo box data stuff. Here's the new version of setResolution:

(note the new for loop at the top)

Code:

void setResolution(HWND hDlg)
{   
    D3DObj.mComboResData.clear();
    D3DClass::resData _resData;
    int j = 0;

    for (int i = 0; i < D3DObj.mResolutionPair.size(); i += 4)
    {        
        D3DObj.mComboResData.push_back(_resData);       
    }

    for (int i = 0; i < D3DObj.mResolutionPair.size(); i += 4)
    {
        _resData.w = D3DObj.mResolutionPair[i];
        _resData.h = D3DObj.mResolutionPair[i + 1];
        _resData.Hz = (double)(D3DObj.mResolutionPair[i + 2]) / D3DObj.mResolutionPair[i + 3];

        std::wstring _resolution = std::to_wstring(_resData.w) +
            L" x " + std::to_wstring(_resData.h);

        const WCHAR* resolution = _resolution.c_str();
        D3DObj.mComboResData[j] = _resData;

        WPARAM nI = (WPARAM)SendMessage(GetDlgItem(hDlg, IDC_COMBO1), CB_ADDSTRING, (WPARAM)0, (LPARAM)resolution);
        SendMessage(GetDlgItem(hDlg, IDC_COMBO1), CB_SETITEMDATA, (WPARAM)nI, (LPARAM) & (D3DObj.mComboResData[j])); // change nI variable to 0 and it works (for first entry)
        j++;       

        #ifdef SKIP  // change to _DEBUG to display
            std::wstring _string = std::to_wstring(_resData.w) + L" "
                + std::to_wstring(_resData.h) + L"\n"
                + std::to_wstring(_resData.Hz) + L"\n"
                + std::to_wstring(nI) + L"\n\n"; // may as well print nI, too
            OutputDebugString(_string.c_str());
        #endif
    }

    return;
}

And that finally...is it. The struct resData is defined in the separate class with initialized variables, so this way of filling up the vector doesn't throw any errors. Then it's just a case of using the typical array indexing on the vector rather than push_back function and that's the end of the matter.

I still don't know exactly why this works but it's obviously the cause and now there are no longer any issues with the indexing, and the code doesn't have to run twice to work.

If anyone can mop up that last little mystery of why this works that would be most helpful.

Thanks

I was thinking that if repeating all the code worked, then the next step would be to find the smallest repeated code that worked. That would then give a clue to a more correct solution. So you got ahead of me there!

Can you upload a zip of the current code that works? I want to see the exact current situation so maybe I can figure out what's going on, since it still doesn't really make sense.

EDIT:
I think I just figured out the problem. I should've thought of it right away.

It is due to the way a vector's dynamic memory works. As a vector grows, it needs to dynamically allocate new memory. In doing so, it may have to move the previously allocated memory to a new location.

The first time through, the vector is empty and so needs to allocate and reallocate memory as it grows, presumably also moving the already-allocated memory at some point, thereby invalidating the pointers that you've stored.
The second time through, the vector already has enough reserve capacity to hold the list, so it's all allocated without any moving and all of your stored pointers work.

The solution is to reserve enough memory at the start. Try this:

Code:

void setResolution(HWND hDlg)
{   
    D3DObj.mComboResData.clear();
 
    // reserve enough memory to avoid relocation
    D3DObj.mComboResData.reserve(D3DObj.mResolutionPair.size() / 4);
 
    for (int i = 0; i < D3DObj.mResolutionPair.size(); i += 4)
    {
        D3DClass::resData resData {
            D3DObj.mResolutionPair[i];
            D3DObj.mResolutionPair[i + 1];
            (double)(D3DObj.mResolutionPair[i + 2])
                   / D3DObj.mResolutionPair[i + 3];
        };
 
        D3DObj.mComboResData.push_back(resData);
 
        std::wstring resolution = std::to_wstring(resData.w)
                       + L" x " + std::to_wstring(resData.h);
 
        HWND hwnd = GetDlgItem(hDlg, IDC_COMBO1);
        WPARAM nI = (WPARAM)SendMessage(hwnd, CB_ADDSTRING, 0,
                                (LPARAM)resolution.c_str());
        SendMessage(hwnd, CB_SETITEMDATA, (WPARAM)nI,
            (LPARAM)&D3DObj.mComboResData.back());
    }
}

Ok John I'll give that a try thanks!

Did you read the edit to my last post? Yeah, no upload needed.

Just saying it's "not successful" is not very useful.
Remember that I can't test it since Windows makes me physically ill.
Does it even compile?
I changed a couple other things that I though would simplify it.
Maybe I made a mistake there.
The main thing is to replace your initial loop with:

Code:

    D3DObj.mComboResData.reserve(D3DObj.mResolutionPair.size() / 4);

So try that with your original code. Your initial loop is basically just doing the same thing (as far as it's doing anything useful).

It compiles alright John yes but it doesn't work. I haven't tried your latest solution. I've kinda had enough of it now. I really just wanna buy you a crate of beer. (Does your local brewer accept Paypal?)

*p.s. oh and I wasn't kidding bout the case of beer John - pm me where to send it and I'll send it over fr

You don't owe me anything, Chris.
I wouldn't have helped if the mystery wasn't somewhat interesting to me.
I'm embarrassed that the problem was just storing invalid pointers due to memory allocation/relocation.
Nothing to do with windows messages.
I should've gotten that right away.
I'm losing it, man.
(I still don't see why my code didn't work, grumble grumble, whatever.)
Anyway, until next time my friendly.

BTW, the code I gave in the previous post wouldn't work unless you also used push_back instead of indexing pre-existing elements.