This implementation uses a line sweep algorithm to copy the precisely the
intersection of all accumulated damage rectangles, ensuring that every
pixel is copied exactly once, and no pixel is ever copied multiple times.
Furthermore, once a row has been swept, we update the framebuffer write
pointer immediately.
Certain drivers do not support pitches that are not multiples of 128 bytes,
and instead just does some kind of rounding internally. On DXGI, this is not
a problem because the API rounds pixel pitch, but NvFBC does not. This causes
certain resolutions to simply not work with dmabuf, most notably 3440x1440,
which is 1440p ultrawide.
Since we are copying pixels with the CPU anyways, we might as well round the
pitch up to 128 bytes (32 pixels).
This commit adds a new host configuration option, nvfbc:diffRes, which
specifies the dimensions of every block in the diff map. This defaults to
128, meaning the default 128x128 block size.
Since block sizes other than 128x128 is not guaranteed to be supported by
NvFBC, the function NvFBCGetDiffMapBlockSize was introduced to query the
support and output the actual block size used.
When our window is destroyed, our timers are also destroyed. This causes our
attempt at destruction to fail. Instead, set MessageHWND to NULL in the
WM_DESTROY handler and don't try destroying the timers if the window is gone.
DestroyWindow can only be invoked on the thread that created the window.
All other threads must use WM_CLOSE or another message to signal tell the
window to destroy itself.
MinGW seems to decide at random whether it wants to use memcpy from
mscvrt.dll or ntdll.dll. Currently, on Debian buster, ntdll.dll is chosen,
while on sid, mscvrt.dll is chosen.
This commit declares a new .def file for ntdll containing only the
functions we want to link from ntdll.dll, and generates ntdll.a from it
with dlltool. This way, MinGW will never be tempted to link functions
like memcpy from ntdll.dll.
This function is sometimes flaky and may fail for no apparent reason,
see https://stackoverflow.com/q/3945003. This has also been experienced
during the development of #610.
This commit adds logging so we may see if it ever fails for no reason
and work out some way to fix it.
We were using an auto-reset event to signal the mousehook exit. This was
fine when there was only one thread, but with the addition of the update
thread, only one thread is signaled, causing the wait to last forever.
The fix is switching to a manual reset event and call ResetEvent after
the threads have exited.
The type of the QuadPart member of the LARGE_INTEGER union is actually
LONGLONG, so we should cast to LONGLONG instead of int.
This avoids truncation should (ms * 10000.0f) exceed 2^31-1.
This function is available since Windows Vista and can therefore be used
directly without going through GetProcAddress. Unfortunately, MinGW does
not have d3dkmthk.h, but we can declare the prototype ourselves and link
against gdi32.dll.
There is no need to LoadLibrary and GetProcAddress to get pointers to
NtDelayExecution or NtSetTimerResolution. These functions don't have
prototypes in any SDK header, but they are exported in ntdll.dll and
we can simply declare the prototype and link ntdll.
There is also no chance that the functions do not exist: I checked an
old install of Windows NT 4.0 and both of these functions exist.
Also used NtSetTimerResolution instead of ZeSetTimerResolution for
consistency (they are the same).
Also changed system timer resolution log message units to μs with
one decimal digit for readability. This is the actual amount of
precision available to us.
According to MSDN documentation for CreateEnvironmentBlock, "[i]f the
environment block is passed to CreateProcessAsUser, you must also
specify the CREATE_UNICODE_ENVIRONMENT flag."
Also pass DETACHED_PROCESS because the host is a GUI application and
doesn't use the console.
Since with the service, we are already running as SYSTEM, we don't need
to use dupeSystemProcessToken to get the token for SYSTEM. This removes
the need for having SeDebugPrivilege, SeTcbPrivilege, and
SeAssignPrimaryTokenPrivilege, or otherwise doing sketchy things.
Furthermore, we now only open the token with the privileges we actually
need.
This allows the process to be terminated without resorting to
TerminateProcess. With some fixes, this allows the notification icon to be
removed when the service is restarted.
Furthermore, instead of sending WM_DESTROY to fool the window into believing
it's being destroyed, we actually call DestroyWindow now.
For adjacent changed regions, we actually use the bounding box for the
entire polygon. This may result in more area being damaged than strictly
necessary, but is nevertheless desirable since it reduces the number of
rectangles.
The windows hook WH_MOUSE_LL is called in such a way that any delay in
processing causes a system wide stall. This change spawns an extra
thread which waits on an event set by the hook which is then used to
call the callback with an artifical limit of 1000Hz.
Before we try and perhaps fail to init DXGI, we should print out what
the device is so that when there is an error report we can immediately
see if the user has the QXL device attached still.
While it's correct for DXGI to use a asyncronous waitFrame model, other
capture interfaces such as NvFBC it is not correct. This change allows
the capture interface to specify which is more correct for it and moves
the waitFrame/post into the main thread if async is not desired.
Before, we only break out of the current row when a change is detected,
and all subsequent rows are still scanned. Now we break out of the entire
loop. This should make change detection ever so slightly faster.
Testing shows that `D3DKMTSetProcessSchedulingPriorityClass` has a
positive performance impact for NvFBC as well as DXGI, as such always
try to boost the priority for the windows host.
This so called "enhanced" event logic is completely flawed and can never
work correctly, better to strip it out and put our faith in windows to
handle the events for us.
And yes, I am fully aware I wrote the utter trash in the first place :)
People often miss the warnings about invalid arguments in their command
line, this last minute patch attempts to address this by making
warnings, errors, fixme's and fatal errors stand out if stdout is a TTY.
This reverts commit d82f2e510d.
While the proposed change is more correct, it breaks the generation of
the file due to failure to locate the resource files, such as
`resources/icon.ico`.
This change adds an average function to time how long it takes the GPU
to copy and map the texture, and then uses this average to sleep for 80%
of this average lowering CPU usage and potentially decreasing lock
contention.
It has been detemined that a failure to init NvFBC causes a 20-30%
performance penalty on non NvFBC supported hardware (GeForce) when using
DXGI, as such reverse the order and default to using DXGI as our first
option.
If NvFBC is still desired, pr #500 added the option `app:capture` which
can be used to force NvFBC.
One of the most common issues reported in the support channels is the
IVSHMEM size being too small. This change adds a calculation to
determine an optimal size and uses the new `os_showMessage` platform
method to display a message box to the user with the error.
Since we now let the mouse hook linger until the process is killed, the
cursor event that the hook signals may now be null, as the capture could
have stopped. If the hook fires during this time, a crash occurs.
Instead of converting every SID to string with ConvertSidToStringSidA
and compare it with the magical SID string for local system with strcmp,
we could instead create the local system SID and compare directly with
EqualSid.
We don't actually have any handles that should be inherited, so specifying
TRUE for bInheritHandles to CreateProcessAsUserA is pointless.
Furthermore, according to MSDN, "[y]ou cannot inherit handles across
sessions," and we are spawning the host in a different session, so this
is even more pointless.
Instead of doing ShellExecute from the service, we instead get the token
of the currently logged in user, and do CreateProcessAsUserA to run
notepad with that token. This should be safe.
Also for failure to parse command line. For these errors, restarting
with exponential backoff will not help: no amount of restarting the
service could possibly make the ivshmem device exist or larger, so
we shouldn't try.
Certain users of Radeon cards have observed that the host fails to start
at boot, with D3D11CreateDevice failing with HSTATUS 0x887a0004, which
translates to "The specified device interface or feature level is not
supported on this system."
This failure results in a LG_HOST_EXIT_FAILED exit code, which the service
does not attempt to restart. The user has to manually restart the service
for the host application to work.
These users reported that the host application started fine on
B2. This strongly suggests that the fix to enable capturing the login
screen made the host application start too early during the boot process,
and the graphics driver did not have time to initialize fully.
This PR allows the service to retry a few times on LG_HOST_EXIT_FAILED,
with exponential backoff, before giving up. This should cover this bug
and other similar bugs related to the early initialization which I do not
have logs for.
This commit introduces a new option, app:capture, which can be set to
either DXGI or NvFBC to force the host application to use that backend.
This is very useful for testing DXGI on Quadro cards, which would default
to running with NvFBC.
This will allow us to add an option to disable the screensaver on the client
when an application in the guest requests it. This behaviour may be useful
when the guest is doing media playback.
The mouse hook code is very fragile, and we would like to avoid unhooking
and re-hooking as much as possible.
After this commit, this is done only once, and the hook and 1x1 window is
only destroyed upon exit. This, of course, comes with the downside of
the slight performance penalty if the guest machine is used directly while
the host is running and the client is not running.
Moving NvFBCToSysSetup to nvfbc_init means that when the pointer thread
fails to be created, NvFBCToSysRelease needs to be called.
To resolve such cleanup issues in the future, we instead call nvfbc_deinit,
which should cleanup everything that needs to be cleaned up. fails.
When NvFBCToSysCapture reports recreation is required, we return
CAPTURE_RESULT_REINIT, which eventually calls nvfbc_deinit and then
nvfbc_init.
However, the NvFBC object is actually created in nvfbc_create, which
means the NvFBC object is never actually recreated. The result is an
endless cycle of NvFBC asking for recreation. This commonly manifests
as the client waiting endlessly for the host when the guest machine
reboots.
In this commit, the NvFBC object creation is moved into nvfbc_init,
and when recreation is required, it will actually be recreated.
mouseHook_install and dwmForceComposition both create threads, but these
are only freed in nvfbc_deinit which is not called if nvfbc_init fails.
These should be freed if the pointer thread fails to be created, as
nothing else could be cleaning it up.
Instead of using %windir%\Temp, which is not accessible by default and
contains a lot of unrelated files, as the location for our log files,
this commit moves it to %ProgramData%\Looking Glass (host), which will
be a dedicated directory just for the LG host log files. This applies
to both the host application logs and the service logs.
Also, we now switched to using PathCombineA from shlwapi.dll instead
of using snprintf, which greatly simplifies the code. PathCombineA
guarantees that the path would not overflow a buffer of MAX_PATH.
Before this commit, the NvFBC backend only generated the first cursor
position update when the mouse moves. Therefore, if the user does
not move the mouse, the cursor will be shown at (0, 0), which is not
ideal.
This commit changes this behaviour to unconditionally generate a
cursor update when the mouse hook initializes.
Before this change, the log is buffered, so if the host application exits
for any reason, it usually would not show up in the log file immediately,
and the service has to be restarted for the logs to be flushed.
This commit disables the buffering so that any log entries shows up
immediately.
This is because we keep track of the top-left corner of the cursor, not
the location of the hotspot. When the cursor shape changes, the hotspot
location may also change. When it does, the position of the top-left
corner changes and requires an update.
In the case that we do not have the current cursor position, which
happens on startup, we do not generate this update.
NvFBC is unable to capture certain applications that bypasses the DWM
compositor, for example, Firefox playing video in full screen. This
has been a known issue for a long time with Nvidia's ShadowPlay, see:
* https://www.nvidia.com/en-us/geforce/forums/geforce-experience/14/233709/
* https://crbug.com/609857
Nvidia won't fix this, but there are workarounds. For example, we
create a transparent 1x1 layered window, which forces desktop composition
to be enabled.
Note that SetLayeredWindowAttributes also supports alpha-based transparency,
but setting transparency to 0 will cause DWM to skip composition. We could
use a transparency of 1, but this ruins the image by the slightest bit,
which is unacceptable. Therefore, we must use chroma key-based
transparency, which tricks DWM into compositing despite being fully
transparent.
Basically, this creates a separate thread for the mouse events, and this
thread detects that the desktop has changed (say to the secure desktop),
and unhooks, switches to the new desktop, and then rehooks.
This allows the cursor location to be updated while using NvFBC on secure
desktop and the login screen.
WTSGetActiveConsoleSessionId will return a session even if it's not logged in,
unlike our old GetInteractiveSessionID function. Launching looking glass on
such a console session will allow the login screen to be captured.
Note that WTSGetActiveConsoleSessionId() will return 0xFFFFFFFF if there are
no sessions attached.
To quote MSDN documentation:
> The lpApplicationName parameter can be NULL, in which case the executable
> name must be the first white space–delimited string in lpCommandLine. If
> the executable or path name has a space in it, there is a risk that a
> different executable could be run because of the way the function parses
> spaces. The following example is dangerous because the function will
> attempt to run "Program.exe", if it exists, instead of "MyApp.exe".
>
> LPTSTR szCmdline[] = _tcsdup(TEXT("C:\\Program Files\\MyApp"));
> CreateProcessAsUser(hToken, NULL, szCmdline, /*...*/ );
>
> If a malicious user were to create an application called "Program.exe" on
> a system, any program that incorrectly calls CreateProcessAsUser using the
> Program Files directory will run this application instead of the intended
> application.
>
> To avoid this problem, do not pass NULL for lpApplicationName.
So instead, we pass the executable to lpApplicationName instead, which avoids
the issue. MSDN says:
> The lpCommandLine parameter can be NULL. In that case, the function uses
> the string pointed to by lpApplicationName as the command line.
This also avoids the strdup since lpApplicationName is LPCSTR unlike
lpCommandLine which is LPSTR.
It shouldn't have any effect, since the host application is created with
the token, and there is no need for the service itself to impersonate.
In practice, removal doesn't appear to have any effect on the ability to
capture privileged things like secure desktop.
Use the process handle returned by CreateProcessAsUserA to wait on the
process. This results in faster response times and less polling.
For example, it now restarts instantly when UAC is activated.
This also removes the call to OpenProcess and rendering the mutex unnecessary.
As a bonus, it should fix#298.
The host process will be changed to return these codes, from which the
service process could decide whether to exit or restart the process and log.
Note that on Windows, return values are 32-bit unlike POSIX which is only 8.
This makes it a compile-time error to call a function that semantically
takes no parameters with a nonzero number of arguments.
Previously, such code would still compile, but risk blowing up the stack
if a compiler chose to use something other than caller-cleanup calling
conventions.
This commit bumps the KVMFR protocol version as it adds additional
hotspot x & y fields to the KVMFRCursor struct. This corrects the issue
of invalid alignment of the local mouse when the shape has an offset
such as the 'I' beam.
