GHOST/WIN32: remove start time offset from getMilliSeconds #117618

Campbell Barton · 2024-01-29T02:06:39+01:00

Campbell Barton commented

2024-01-29 02:06:39 +01:00

This isn't necessary and has been removed from macOS & X11,
Wayland never did this.

Besides removing the offset GetTickCount() has been replaced by
GetTickCount64 to prevent 32bit rollover when high resolution timers
aren't supported.

GetTickCount64 is supported by Windows Vista and newer.

This isn't necessary and has been removed from macOS & X11, Wayland never did this. Besides removing the offset GetTickCount() has been replaced by GetTickCount64 to prevent 32bit rollover when high resolution timers aren't supported. --- GetTickCount64 is supported by Windows Vista and newer.

Campbell Barton commented

2024-01-29 02:07:11 +01:00

@blender-bot build windows

Campbell Barton force-pushed pr-win32-time-offset from c4a5970132 to c2390b4da5

2024-01-29 02:11:17 +01:00

Compare

Campbell Barton requested review from Harley Acheson 2024-01-29 02:26:32 +01:00

Campbell Barton force-pushed pr-win32-time-offset from c2390b4da5 to dcc54ad7c9

2024-01-29 02:28:27 +01:00

Compare

Campbell Barton requested review from Ray molenkamp 2024-01-29 02:30:40 +01:00

Harley Acheson commented

2024-02-05 22:24:17 +01:00

@ideasman42

Sorry for the delay in looking at this.

With this PR we are saying that for these message times we don't care what the actual values are as we only care about the differences between them. So we can have the values be an amount of time since the computer was turned on, rather than from when blender was started. That does simplify things.

The only thing I noticed while testing this is a problem that already exists in that getMessageTime so it is hard to change that without fixing that too.

On Blender startup we currently call getMessageTime four times or so before we get any Windows messages in our queue. In that state the return of GetMessageTime() is zero. GetTickCount still gives a proper value so we end up calculating a delta that is a negative value equal to the number of ticks since the system was started.

I think we need that function to look something like?

static uint64_t getMessageTime(GHOST_SystemWin32 *system)
{
  /* Get difference between last message time and now. */
  int64_t t_delta = 0;
  int64_t message_time = GetMessageTime();
  if (message_time > 0) {
    t_delta = message_time - GetTickCount();
    /* Handle 32-bit rollover. */
    if (t_delta > 0) {
      t_delta -= int64_t(UINT32_MAX) + 1;
    }
  }

  /* Return message time as 64-bit milliseconds with the delta applied. */
  return system->getMilliSeconds() + t_delta;
}

@ideasman42 Sorry for the delay in looking at this. With this PR we are saying that for these message times we don't care what the actual values are as we only care about the differences between them. So we can have the values be an amount of time since the computer was turned on, rather than from when blender was started. That does simplify things. The only thing I noticed while testing this is a problem that already exists in that `getMessageTime` so it is hard to change that without fixing that too. On Blender startup we currently call `getMessageTime` four times or so before we get any Windows messages in our queue. In that state the return of GetMessageTime() is zero. `GetTickCount` still gives a proper value so we end up calculating a delta that is a negative value equal to the number of ticks since the system was started. I think we need that function to look something like? ``` static uint64_t getMessageTime(GHOST_SystemWin32 *system) { /* Get difference between last message time and now. */ int64_t t_delta = 0; int64_t message_time = GetMessageTime(); if (message_time > 0) { t_delta = message_time - GetTickCount(); /* Handle 32-bit rollover. */ if (t_delta > 0) { t_delta -= int64_t(UINT32_MAX) + 1; } } /* Return message time as 64-bit milliseconds with the delta applied. */ return system->getMilliSeconds() + t_delta; } ```

Ray molenkamp commented

2024-02-05 22:26:54 +01:00

ideally getMessageTime shouldn't be called unless we're handling a message, where do those 4 calls originate? can we eliminate them?

ideally `getMessageTime` shouldn't be called unless we're handling a message, where do those 4 calls originate? can we eliminate them?

Harley Acheson commented

2024-02-05 23:30:50 +01:00

When we are creating the window, so within GHOST_WindowWin32() but near the very end of it, we call ::UpdateWindow(m_hWnd); with a comment of "Force an initial paint of the window" (GHOST_WindowWin32.cc at around line 209). This causes use to get WM_ACTIVATE, WM_SIZE, WM_MOVE, and then WM_PAINT. While processing these messages the return of GetMessageTime () is zero.

Yes, there is nothing that looks weird if UpdateWindow() is not called.

When we are creating the window, so within `GHOST_WindowWin32()` but near the very end of it, we call `::UpdateWindow(m_hWnd);` with a comment of "Force an initial paint of the window" (GHOST_WindowWin32.cc at around line 209). This causes use to get WM_ACTIVATE, WM_SIZE, WM_MOVE, and then WM_PAINT. While processing these messages the return of GetMessageTime () is zero. Yes, there is nothing that looks weird if UpdateWindow() is not called.

Harley Acheson commented

2024-02-06 00:06:49 +01:00

@LazyDodo - So we could take this PR as it is, and remove that UpdateWindow() separately.