GPUOpen-LibrariesAndSDKs / RenderPipelineShaders

Render Pipeline Shaders SDK
MIT License
312 stars 24 forks source link

rpsl_explorer FileMonitor does not notify on file change #33

Open ChemistAion opened 1 year ago

ChemistAion commented 1 year ago

There is a missing SHCNRF_ShellLevel flag here - in the type of events for which to receive notifications: https://github.com/GPUOpen-LibrariesAndSDKs/RenderPipelineShaders/blob/31183408385c1e3e1711bd0320e17fcdf1ee07e9/tools/rpsl_explorer/src/file_monitor.hpp#L36-L41 It is necessary to include it to capture changes made, e.g. from text-editors (considered as a ShellLevel source).


Other, minor things:


Please find my snipped with corrected version:

static constexpr UINT UM_FILE_CHANGED = WM_USER + 4097;

bool BeginWatch(HWND hWndListener, const std::string& fileName)
{
    auto iter = m_registerIDs.find(fileName);

    if (iter != m_registerIDs.end())
    {
        return false;
    }

    PCIDLIST_ABSOLUTE pidl = ILCreateFromPathA(fileName.c_str());

    if (pidl == 0)
    {
        return false;
    }

    SHChangeNotifyEntry notifyEntry = { pidl, FALSE };

    ULONG uId = SHChangeNotifyRegister
    (
        hWndListener,
        SHCNRF_InterruptLevel | SHCNRF_ShellLevel | SHCNRF_NewDelivery,
        SHCNE_RENAMEITEM | SHCNE_DELETE | SHCNE_UPDATEITEM,
        UM_FILE_CHANGED,
        1,
        &notifyEntry
    );

    if (uId != 0)
    {
        m_registerIDs.emplace(fileName, uId);
    }

    return (uId != 0);
}
FlorianHerickAMD commented 1 year ago

Which text-editors did you experience this with? For example, updates from Notepad++ seem to work just fine on my side.

ChemistAion commented 1 year ago

@FlorianHerickAMD: Latest Notepad++ v8.4.9 x64 on Windows 10 Pro for Workstations (10.0.19045 Build 19045)

Additionally, I have conducted a second test on a different machine, and the results align with the previous findings. Please see the comment below.

ChemistAion commented 1 year ago

@FlorianHerickAMD: I can also confirm this by testing with the latest Notepad++ v8.4.9 x64 on Windows 11 Enterprise (10.0.22621 Build 22621). RPS from fresh repo clone, builded with latest VS Community 2022 (17.6.2).

Scenario to reproduce:

Root cause: UM_FILE_CHANGED message is not generated due to a missing SHCNRF_ShellLevel flag in SHChangeNotifyRegister(...) - as described in the first comment.

ChemistAion commented 1 year ago

The issue persists in both Debug and Release configurations.