Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Software drawing v2 #733

Closed
wants to merge 3 commits into from
Closed

Software drawing v2 #733

wants to merge 3 commits into from

Conversation

akallabeth
Copy link
Member

Fixed some issues found in #728:

  • Channel cleanup
  • Code cleanup to make it easier to read
  • Issues found by clang scan-build fixed

@antenore
Copy link
Member

@akallabeth wow!

@giox069 have tested your PR, but due to some libfreerdp issues, it crashes when the address sanitizer is ON, so we must hold on this impressive PR at the moment.

Would you mind trying to compile both with the address sanitizer and see where the issue is?

Thanks again!

@akallabeth
Copy link
Member Author

@antenore I've run this pull with address sanitizer yesterday, what issues do you experience?

@akallabeth
Copy link
Member Author

@antenore The only issue I found is FreeRDP/FreeRDP#3076 which is currently not merged.
There are some drawing artifacts, but that is a FreeRDP problem and not of your concern at the moment.

@giox069
Copy link
Contributor

giox069 commented Jan 22, 2016

@akallabeth: I can confirm that without Freerdp PR 3076 the address sanitizer crashes. But now I'm having problems compiling this PR against PR 3076 (missing header file ?)... I need some more time to test.

/home/giovanni/remmina_devel/Remmina/remmina-plugins/rdp/rdp_plugin.h:50:29: fatal error: winpr/clipboard.h: No such file or directory

@giox069
Copy link
Contributor

giox069 commented Jan 22, 2016

and @antenore: please don't merge this PR until we have FreeRDP's PR3076 merged AND updated the freerdp on the remmina-next PPA, or we will break remmina on some thousand of machines :)

@akallabeth
Copy link
Member Author

As for my changes to remmina_rdp_close_connection anyone knows a more elegant approach to prevent the dead lock there? Don't really feel comfortable starting a new thread just to clean up resources.

@antenore
Copy link
Member

Sorry, I've generated some conflicts. Let me know if I can help.

Armin Novak added 3 commits January 22, 2016 15:00
@akallabeth
Fixed leaks and reset issues detected by clang scan-build.
f7a2657
@akallabeth
Refactored the RDP client using only FreeRDP drawing.
240ec53 
Removed the custom drawing primitives, now soly relying on
FreeRDP drawing primitives. This way support for every new
FreeRDP drawing feature is immediately available to remmina.
@akallabeth
Fixed pointer cleanup.
f5fffd8 
There was a deadlock when cleaning up the pointer cache.
The cleanup code did want to post an event to the event queue
but it was locked due to the cleanup code being run in that thread.
@giox069 giox069 mentioned this pull request Mar 3, 2016
Merged
@antenore
Copy link
Member

@akallabeth I'm closing this because the code has diverged too much.
We have the code anyway.
Thanks a lot!

@antenore antenore closed this Apr 13, 2018
@akallabeth akallabeth deleted the software_drawing_v2 branch April 15, 2018 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@akallabeth @antenore @giox069