mirror of
https://github.com/quasar/Quasar.git
synced 2026-04-25 15:25:59 +03:00
[GH-ISSUE #298] Remote Desktop Bug #147
Labels
No labels
bug
bug
cant-reproduce
discussion
duplicate
easy
enhancement
help wanted
improvement
invalid
need more info
pull-request
question
wont-add
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/Quasar#147
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @ghost on GitHub (Jul 27, 2015).
Original GitHub issue: https://github.com/quasar/Quasar/issues/298
Originally assigned to: @MaxXor on GitHub.
I believe this is why the InvalidOperationExceptions are thrown. The
UnsafeStreamCodecis trying to use the image at the same time theFrmRemoteDesktopis accessing the image.This post explains the best solution of restricting access to the object to one thread
http://stackoverflow.com/a/1852451
Also, the Remote Desktop can get really laggy when it is maximized, or if the remote client's resolution is larger than the picturebox. Could the issue be caused by the
SendBlockingmethod? ofGetDesktoppackets?@MaxXor commented on GitHub (Jul 27, 2015):
Oh yea, the first bug is not nice.. I think the problem is the server which receives the new screen images too fast, I tried adding some locks to it, but seems it didn't change anything. Btw. the
UnsafeStreamCodecdoesn't use the same image which we use inFrmRemoteDesktop, because we are cloning it here. Also for me the screen is sometimes heavily flickering when I first start the Remote Desktop.I'll look into this further.
@yankejustin commented on GitHub (Jul 27, 2015):
@MaxXor That is actually incorrect. Using
Clone()on aBitmapdoes provide you with a newBitmapobject with its own copied values. However, pixel data is shared with the original object.@yankejustin commented on GitHub (Jul 27, 2015):
Also, we should dispose of
picDesktop'sImage. CallingDispose()on this object isn't going to really help us much because we are actually not disposing ofpicDesktop'sImage.To end the wall of text, I would also like to point out that we should add
picDesktop.SuspendLayout()before changing the image, andpicDesktop.ResumeLayout()after changing the image. This should remove the flicker effect.@MaxXor commented on GitHub (Jul 27, 2015):
@yankejustin Thanks, didn't know about this. I think the code should now create a deep copy of the bitmap. I think it does dispose the old bitmap from the picturebox correctely, because we were just changing the picturebox image before, but didn't take care of the old image, so we had to wait till the GC collects it.
I think it's still not 100% fixed, anyways I'll also take a look at the high cpu usage when Remote Desktop window is maximized, it's very slow.
@yankejustin commented on GitHub (Jul 27, 2015):
@MaxXor There is actually another issue I noticed. There is quite a significant amount of lag that aggregates when using Remote Desktop now. It builds up (got it to ~25 seconds of lag in under 2 minutes) and causes the user to be unable to pause the changing of
picDesktop.Image.@MaxXor commented on GitHub (Jul 27, 2015):
@yankejustin Do you also have Remote Desktop window maximized? For me there's no lag when not changing the window size.
@yankejustin commented on GitHub (Jul 27, 2015):
I had it maximized at some point, but it will lag for me regardless. It seems as if the Client still continues to send the changed image and the Server will continuously handle it... Even if I press the Stop button.
@MaxXor commented on GitHub (Jul 27, 2015):
@yankejustin Can you check if the lag gets worse when you just keep the window size?
@yankejustin commented on GitHub (Jul 27, 2015):
@MaxXor It seems that increased movement on the Client (more changing pixels) results in increasing lag.
@MaxXor commented on GitHub (Jul 27, 2015):
I'm working on a fix for this.
@ghost commented on GitHub (Jul 27, 2015):
@MaxXor, while you are working on this, could you also add this to the
ScreenHelperclass? It makes transparent areas visible, so you are able to interact and see more of the desktop when it draws the image http://puu.sh/jfgRh/3458ae66a6.pngAlso, I'm 100 percent sure its because the object is being accessed at the same time, because I'm getting an error that says that. This happens in multiple scenarios. Recreating these scenarios can be tough because you have to get lucky you time them right. Try dragging the border of the remote desktop with mouse input enabled, and quickly moving the mouse over the picturebox, after moving the edge of the form so the width expands. Eventually it will throw an exception stating that the object is in use somewhere else. There are three methods (the MouseDown, MouseUp, MouseMove events) that calculate the size of the image to display in the picturebox and this accesses the image that is being invoked rapidly, and also the image that is being decoded in the
UnsafeStreamCodecclass.I had both of these classes throw the error, so one of them is accessing the object first simultaneously
Hope you fix it! :D
@ghost commented on GitHub (Jul 27, 2015):
Also, before I scurry off for sleep xD, The lag I was experiencing would also happen if the Remote Desktop form was smaller than the resolution of the client machine. This was also causing lag but it might have just been placebo. I would like to think its because the
GetDesktopresponse packet is being sent to the server bySendBlockingmethod. Not 100 percent on that but I hope you guys can figure it out!https://github.com/MaxXor/xRAT/blob/master/Client/Core/Packets/ClientPackets/GetDesktopResponse.cs#L35
@yankejustin commented on GitHub (Jul 27, 2015):
@d3agle Thank you for the input!
And yes, the pixel data is very likely being accessed in different places and manipulated. This is because (as I stated above) the
Clone()method of aBitmapactually will still share pixel data with the original.@yankejustin commented on GitHub (Jul 27, 2015):
@d3agle Read your other comment a second time and realized the new(ish)
SendBlockingmethod. Why do we send these packets in a blocking fashion? Why can't it be in a way similar to UDP packets and just send the data? This makes sure that we are using the latest packets from the Client.@ghost commented on GitHub (Jul 28, 2015):
@yankejustin, Thank you for the explanation. I see how that would make a world of difference in responsiveness.
Here is an example of the lag. I hope that this link will help provide some insight on to why this might be happening. http://i.gyazo.com/b8abcefab48a2acdc2042701045e327e.mp4
The Client machine has a resolution of 800x600. Notice when I move the forms width a smidge, the lag shortly ensues 😞
Here is another example that might provide some insight:
http://i.gyazo.com/9baed86777be1ef890e99cf5f93d6eaa.mp4
Notice how the Client is responsive with the data that is being sent. It appears only the server is affected
@ghost commented on GitHub (Jul 28, 2015):
@MaxXor, the deep copying of the Bitmap object seems to be working for me. I've been trying to break it for the past hour and haven't been successful.'
Edit: It has crashed only once, so it has definitely decreased the frequency
@ghost commented on GitHub (Jul 28, 2015):
I believe this post is relevant to the "lag" we are experiencing.
http://stackoverflow.com/a/14282467
Here is a viable solution to the lag problem:
http://stackoverflow.com/a/3567824
The lag might be due to the picturebox control drawing the image, so when we stretch the form to a size that doesn't fit the aspect ratio of the bitmap, the control has to spend more time drawing the image, and this could cause it to delay substantially
@MaxXor commented on GitHub (Jul 28, 2015):
@d3agle
CAPTUREBLTcauses my mouse cursor on Windows 7 to disappear whenBitBltis called. Yes, only the Server is affected. I've tried now resizing the image before updating the picturebox, but when the window is maximzed the lag still occurs. 😟Some info: https://stackoverflow.com/questions/11020710/is-graphics-drawimage-too-slow-for-bigger-images
@yankejustin The
SendBlockingis important (also for File transfers) because the bytes of the next image shouldn't get allocated until first one is sent. Without this, it would pump the memory with the images.Are there any problems now when the window is not resized (no lag)?
@ghost commented on GitHub (Jul 28, 2015):
@MaxXor, I see what you mean my mouse is flickering. I wonder if there are any other ways to do something similar.. without it, working on machines (Windows 10 for example) might be difficult :(
Also, perhaps we could try using a control other than the PictureBox to draw the image?
@ghost commented on GitHub (Jul 28, 2015):
It is quite a bit slower after
7e8693ba34(without resizing form)I don't remember any problems with screen capturing in the RELEASE 4 version, no problems with maximizing the form or any delays
@MaxXor commented on GitHub (Jul 28, 2015):
@d3agle In R4 the Remote Desktop was actually slow, the server did always send a
DesktopReceivedand then the client answered with the next image, like a ping pong.@ghost commented on GitHub (Jul 28, 2015):
@MaxXor, That is true. But now, the server is being bombarded with packets, and the terrible picturebox optimizations limit us :(
Do you think it would be viable to create our own control so we have more control of the way the OnPaint event processes images?
I found this while looking for custom controls. http://www.apharmony.com/software-sagacity/2014/05/improving-rendering-speed-in-the-c-picturebox/
@MaxXor commented on GitHub (Jul 28, 2015):
@d3agle I guess yes, we'll need to change how the image gets drawn to the form. (with directx/sharpdx)
https://stackoverflow.com/questions/11030706/c-sharp-fast-pixel-rendering
@yankejustin commented on GitHub (Jul 28, 2015):
I'm working on this.
@yankejustin commented on GitHub (Jul 28, 2015):
@MaxXor Why don't we flush the
Bitmapqueue after the Stop Button is pressed? When it is not cleared, we get the stream of residual pictures that is likely undesirable. Perhaps it should be an option to queue the images together, or to flush the queue on stopping of image updating.@ghost commented on GitHub (Jul 29, 2015):
As suggested in this article, we could only have the server update the picturebox when something on the screen has changed. http://bobcravens.com/2009/04/create-a-remote-desktop-viewer-using-c-and-wcf/
Two main points that were suggested:
The algorithm used to generate the screen capture must be fast.The amount of data shuttled across the network must be minimized.Considering this problem wasn't apparent in the RELEASE 4 version of xRAT, albeit the performance was slower, the ping-pong effect of the packets and response to the packets provided a bottleneck for the control to be able to update itself appropriately. It would make sense that constraining the amount of traffic would be a more ideal solution to this problem. (I have had upwards of 100-200 fps with lower quality settings, which is very nice, but seems a bit extreme for .NET code)
However, if we were to pursue having an impressive frame rate, it seems it would be more ideal to work with SharpDX, provided no other solutions appear.
@MaxXor commented on GitHub (Jul 29, 2015):
@yankejustin I think it's only needed to clear the queue when the form closes.
@d3agle @yankejustin Do you think we should add this "ping pong" back?
@MaxXor commented on GitHub (Jul 29, 2015):
I've created a new branch called
remote-desktop, thats the old Remote Desktop with some improvements from the new one. You can check it out and see the FPS is a lot lower. But I also don't want to add SharpDX or any other DirectX wrapper.I think we should use the old one...
@ghost commented on GitHub (Jul 29, 2015):
I think we should use the old system too. I just installed Windows 10 on my host and I'm getting a steady 27 fps streaming from my second monitor (At 75% quality).
@MaxXor commented on GitHub (Jul 29, 2015):
@d3agle I think that's a good solution, gonna merge the changes now.