RenderPipelineShaders icon indicating copy to clipboard operation
RenderPipelineShaders copied to clipboard

rpsl_explorer FileMonitor does not notify on file change

Open ChemistAion opened this issue 2 years ago • 4 comments

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:

  • UM_FILE_CHANGED should be type of UINT;
  • result of ILCreateFromPathA is not checked (in case of 0);
  • registered-to-notify file is not placed into m_registerIDs - so it would be wrongly considered again

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);
}

ChemistAion avatar May 28 '23 23:05 ChemistAion

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

FlorianHerickAMD avatar May 30 '23 08:05 FlorianHerickAMD

@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 avatar May 30 '23 12:05 ChemistAion

@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:

  • from rpsl_explorer I have opened `examples\hello_triangle.rpsl';
  • this trigers compilation/reloads jit-module etc.;
  • the same file is opened from Notepad++;
  • making/saving changes does not retrigers jit-module;
  • removing tmp cache-dir during these tryouts also makes no difference;
  • note: no fancy path for RPS was used, here: d:\_rps;

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 avatar May 30 '23 12:05 ChemistAion

The issue persists in both Debug and Release configurations.

ChemistAion avatar May 30 '23 16:05 ChemistAion