pysdl2 example for issue #324 #379

neilmunday · 2017-07-30T21:43:23Z

Hi,

I have created a simple pysdl2 example that uses cefpython3 for issue #324

Please feel free to included it in your examples.

Cheers,

Neil.

cztomczak

Thanks for this example. I have provided many comments for style issues that need to be addressed. There is also a question about copyright and licensing of this code. Also it would be nice if you could provide me quick instructions for installing pysdl2 on my Linux machine. Thanks!

cztomczak · 2017-07-31T06:29:00Z

examples/pysdl2.py

+#
+# Tested under Fedora Linux (x86_64).
+#
+# by Neil Munday (www.mundayweb.com)


Do you agree to release your code under BSD-3 new license that cefpython uses? You can add your name to the Authors file in the root directory along with your email or website. The copyright messages in cefpython source files are using this template:

# Copyright (c) {YEAR_OF_CREATION} CEF Python, see the Authors file. # All rights reserved. Licensed under BSD 3-clause license. # Project website: https://github.com/cztomczak/cefpython

The examples from the examples/ directory do not contain any copyright messages and this is intentional, because these examples are intended to be edited freely by users.

When your PR is accepted you will also appear on the Contributors page:
https://github.com/cztomczak/cefpython/graphs/contributors

I am happy for the code to be used freely as people see fit.

cztomczak · 2017-07-31T06:29:26Z

examples/pysdl2.py

+#
+# Only handles mouse events but could be extended to handle others.
+#
+# Requires pysdl2 (and SDL2 library).


Can you please provide quick instructions on installing pysdl2?

pip2 install PySDL2

Note: you will need to make sure you have installed the SDL2 libraries for your OS.

cztomczak · 2017-07-31T06:29:38Z

examples/pysdl2.py

@@ -0,0 +1,182 @@
+#!/usr/bin/python2


This can be removed.

cztomczak · 2017-07-31T06:30:28Z

examples/pysdl2.py

@@ -0,0 +1,182 @@
+#!/usr/bin/python2
+
+#


Remove that kind of comments with following empty line.

cztomczak · 2017-07-31T06:30:43Z

examples/pysdl2.py

+
+#
+# Simple SDL2 / cefpython3 example.
+#


Remove the blank lines in this block of comment.

cztomczak · 2017-07-31T06:35:01Z

examples/pysdl2.py

+		if element_type == cef.PET_VIEW:
+			data = paint_buffer.GetString(mode="rgba", origin="top-left")
+			image = Image.frombuffer('RGBA', (self.__width, self.__height), data, 'raw', 'BGRA')
+			#


Unnecessary.

cztomczak · 2017-07-31T06:35:27Z

examples/pysdl2.py

+			surface = sdl2.SDL_CreateRGBSurfaceFrom(pxbuf, self.__width, self.__height, depth, pitch, rmask, gmask, bmask, amask)
+
+			if self.texture:
+				sdl2.SDL_DestroyTexture(self.texture)


Please use 4x spaces for indentation (pep8).

cztomczak · 2017-07-31T06:36:05Z

examples/pysdl2.py

+	browserHeight = height - headerHeight
+	browserWidth = width
+
+	WindowUtils = cef.WindowUtils()


Too many blank lines between for the next several lines.

cztomczak · 2017-07-31T06:36:21Z

examples/pysdl2.py

+
+	sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO)
+
+	window = sdl2.video.SDL_CreateWindow('cefpython3 SDL2 Demo', sdl2.video.SDL_WINDOWPOS_UNDEFINED, sdl2.video.SDL_WINDOWPOS_UNDEFINED, width, height, 0)


Wrap lines to 79 characters.

cztomczak · 2017-07-31T06:37:27Z

examples/pysdl2.py

+	cef.Shutdown()
+	print "exited"
+
+if __name__ == "__main__":


Please put here only a call to main(), and define main at the top of the file just below the imports. That's how it is done in all other examples, so it should be consistent in this example as well.

neilmunday · 2017-07-31T06:47:52Z

Hi. Thanks for the feedback. I'll submit a modified version of the example later.

neilmunday · 2017-08-01T06:25:24Z

New changes committed as requested.

cztomczak

Hi, it looks good, although there are still some issues.

I've tested using Ubuntu 14.04 / pysdl2 0.9.5 and it works fine - can be added to top comment.

Installation instructions for Ubuntu - this could be added to top comment as well:

sudo apt-get install libsdl2-dev
sudo pip install pysdl2

The keyboard doesn't work at all. All examples in the examples/ directory are officially supported and they need to be complete. Both mouse and keyboard support is required. How hard would it be to add keyboard support?

I will still have to test it on Windows and Mac.

cztomczak · 2017-08-01T07:33:30Z

examples/pysdl2.py

+   pip2 install PySDL2
+
+ Tested configurations:
+ - SDL2 2.0.5 with PySDL2 0.9.3 on Fedora 25 (x86_64)


Great that you've added this.

cztomczak · 2017-08-01T07:33:42Z

examples/pysdl2.py

+    import sdl2
+    import sdl2.ext
+except ImportError:
+    print "SDL2 module not found - please install"


cztomczak · 2017-08-01T07:34:29Z

examples/pysdl2.py

+
+    def OnLoadingStateChange(self, browser, is_loading, **_):
+        if not is_loading:
+            print "loading complete"


Your code should be Python 3 compatible. There is no print statement in Py3.

Are you able to test it using Python3? If not, not a big issue I can do this, just fix the most obvious issues like the print statement.

cztomczak · 2017-08-01T07:35:53Z

examples/pysdl2.py

+        cef.PostTask(cef.TID_UI, exit_app, browser)
+
+class RenderHandler(object):
+    """


Thanks for adding comments like this.

cztomczak · 2017-08-01T07:36:33Z

examples/pysdl2.py

+                'raw',
+                'BGRA'
+            )
+            #


That blank comment can be removed and the one further.

cztomczak · 2017-08-01T07:36:58Z

examples/pysdl2.py

+    print "exited"
+
+def main():
+    # the following variables control the dimensions of the window


Comments should start from upper case letter - there are more fixes like this required.

cztomczak · 2017-08-01T07:37:18Z

examples/pysdl2.py

+    # and browser display area
+    width = 1024
+    height = 768
+    headerHeight = 0 # useful if for leaving space for controls at the top of the window (future implementation?)


Wrap comments to 79 characters as well.

cztomczak · 2017-08-01T07:43:38Z

examples/pysdl2.py

+    cef.Shutdown()
+    print "exited"
+
+def main():


The main() function should go to the top just below imports.

The main() function also seems very long, could this be split into more functions like for example: initialize(), embed_browser() and message_loop() - these would be called from main()?

It could be split up, but the functions would only be called from main() and quiet a few of the variables would have to become global.

neilmunday · 2017-08-02T18:36:08Z

On the keyboard event front, I did try adding support for this in the original version, but I got stumped trying to send the key events to cefpython3. I couldn't work out from the docs how to translate the SDL2 key event characters to ones expected by cefpython3 despite reading the docs. I'll have another look though.

cztomczak · 2017-08-02T18:44:25Z

Keyboard support can be tricky. Take a look at the kivy_.py example. This might be strange, but setting "windows_key_code" on Linux is the way to go.

neilmunday · 2017-08-03T05:27:30Z

I've made progress thanks to digging through the kivy example, and now have key events working for ASCII characters. Just need to handle modifiers and some special keys.

cztomczak · 2017-08-03T05:33:24Z

Great to hear that.

neilmunday · 2017-08-03T21:01:33Z

I've now added keyboard support. Please feel free to test.

cztomczak

Thanks for adding keyboard support. A few issues, see my review comments for details:

OnLoadError should be handled differently
An error when calling exit_app
Some more info on keyboard/mouse support is required in comments
Mouse move events and mouse scroll events do not work. Add at least comments explaining what needs to be done. See my comments.
getKeyCode should be a dict mapping. Users may be extending this, so let's get rid of those ifs to make it easier for them.

cztomczak · 2017-08-04T03:32:10Z

examples/pysdl2.py

+                            1
+                        )
+            elif event.type == sdl2.SDL_MOUSEMOTION:
+                if event.button.y > headerHeight:


Why this condition? Why aren't you sending all mouse move events? When I hover over a link at Google I don't see link being underlined, because mouse move event is not being sent.

Is there available event.position.x/y or similar? If so implementing move events should be easy.

As stated in the comments in the main() function, headerHeight has been defined for use to create a toolbar at the top of the window as a future exercise. SDL unlike other toolkits doesn't provide any kind of GUI widgets - the user has to write their own which as you can imagine can be quite complex. I therefore left this variable in so that mouse events that occur in the region of the window used to paint the cef browser are sent to cef. I could of course remove the headerHeight stuff entirely, but this assumes that the entire window is under cef's control. Let me know what you think.

I meant why only mouse event is sent when there is a click. I see your other comment saying you will add these mouse move events, great!

Ah ha, I see I am trapping mouse motion events for SDL but processing it as a button press. Oops. That's a bug on my part!

cztomczak · 2017-08-04T03:35:43Z

examples/pysdl2.py

+        if not frame.IsMain():
+            return
+        print("Failed to load %s" % failed_url)
+        cef.PostTask(cef.TID_UI, exit_app, browser)


Looks like an error here, wrong number of args. Got this exception:

Failed to load http://www.speedtest.net/pl/ [CEF Python] ExceptHook: catched exception, will shutdown CEF Traceback (most recent call last): File "task.pyx", line 57, in cefpython_py27.PyTaskRunnable (cefpython_py27.cpp:33940) TypeError: exit_app() takes no arguments (1 given)

When displaying "Failed to load %s" it would be nice to display also error_code. I currently have no idea what happened while loading that speedtest.pl website. Is it really necessary to exit app here on error? I would just display a message that url failed and an error code for a reason why it failed. A website may send 404 error, but it will display some webpage and it should be displayed in browser and not exited.

You are right, the app doesn't have to exit here. As mentioned above in another comment, SDL doesn't provide any GUI widgets - it's quite low level in comparison to other GUI toolkits. In order to display an error in the window I would have to either implement some kind of message box or clear the renderer and render text to the window. The latter would be quite simple. However, at that the point without any controls for the user to use they wouldn't be able to go back to the previous page. Is showing errors in the console ok for now?

Yes, showing errors in console is okay. Web page even with error code might contain important information and should be displayed.

cztomczak · 2017-08-04T03:38:41Z

examples/pysdl2.py

+
+def getKeyCode(key):
+    """Helper function to convert SDL2 key codes to cef ones"""
+    if key == sdl2.SDLK_RETURN:


This mapping could be done using a dict:

map_keys = { sdl2.SDLK_RETURN: 13, sdl2.SDLK_DELETE: 46, ... ]

Key mappings is incomplete, so this code will probably be extended by user, so let's get rid of those ifs to make it more clear.

cztomczak · 2017-08-04T03:39:19Z

examples/pysdl2.py

+        return 36
+    if key == sdl2.SDLK_END:
+        return 35
+    raise Exception("Invalid key")


It should say that Keyboard mapping is incomplete and should be extended instead of "Invalid".

cztomczak · 2017-08-04T03:42:49Z

examples/pysdl2.py

+"""
+ Simple SDL2 / cefpython3 example.
+
+ Only handles mouse events but could be extended to handle others.


This needs to be updated. It should say that keyboard and mouse support is incomplete:

Keyboard support may be extended to support:

marking text in inputs with the shift key

selecting all text with ctrl+a

not all special keys may be supported, but can be extended by modifying the getKeyCode() function

Mouse support may be extended to support:

mouse hover by calling SendMouseMoveEvent()

mouse wheel scrolling by calling SendMouseWheelEvent()

I will add mouse move etc. events and update the comments.

cztomczak · 2017-08-04T03:46:30Z

examples/pysdl2.py

+
+    sudo pip2 install PySDL2
+
+ Tested configurations:


Let's put Tested configurations just below "Simple SDL2 / cefpython3 example." text. There are many instructions and notes and list of tested configurations is more important to display first.

You've missed that comment. Let's move "Tested configurations" further up.

neilmunday · 2017-08-04T06:52:44Z

Please see inline comments above.

cztomczak · 2017-08-04T16:19:05Z

It will be great to have this example complete and working. Currently there is only screenshot.py very basic example that shows how to use OSR mode. There is the kivy_.py example, but it needs a big refactoring before being moved to the examples/ directory. You're doing great job.

neilmunday · 2017-08-04T18:11:44Z

Cheers. I originally looked at cefpython3 as I wanted to use it as a basis for an application on the Raspberry Pi. If an ARM version of cefpython3 needs testing in the future, let me know :-)

cztomczak · 2017-08-05T04:10:18Z

I guess you've seen Issue #267. It says what changes need to be done to the build scripts to build CEF Python on ARM. Some more modifications may also be required in other tools and in cefpython's source code. There was one PR already, but with issues.

cztomczak

This looks very good now, found just one minor style issue. I will test it on Windows/Mac and then merge it.

cztomczak · 2017-08-05T04:15:00Z

examples/pysdl2.py

+
+ Where possible SDL2 events are mapped to CEF ones. Not all keyboard
+ modifiers are handled in this example but these could be
+ add by the reader (if desired).


Please add some example of what what doesn't yet work:

marking text in inputs with the shift key selecting all text with ctrl+a

cztomczak · 2017-08-05T04:15:41Z

examples/pysdl2.py

+
+    sudo pip2 install PySDL2
+
+ Tested configurations:


You've missed that comment. Let's move "Tested configurations" further up.

cztomczak · 2017-08-05T04:16:08Z

examples/pysdl2.py

+ be extended by create some simple SDL2 widgets. An example of
+ widgets made using PySDL2 can be found as part of the Pi
+ Entertainment System at:
+ https://github.com/neilmunday/pes/blob/master/lib/pes/ui.py


Thanks for such comments, that is useful.

cztomczak · 2017-08-05T04:17:12Z

examples/pysdl2.py

+    if key in keyMap:
+        return keyMap[key]
+    # Key not mapped, raise exception
+    print("Keyboard mapping incomplete: unsupported SDL key %d. See https://wiki.libsdl.org/SDLKeycodeLookup for mapping." % key)


Wrap this line at 79 characters.

cztomczak · 2017-08-05T04:40:47Z

Put your name and email (or website) to the Authors file (the root dir).

neilmunday · 2017-08-05T08:22:21Z

Hopefully all done now :-)

cztomczak

Looks good :)

cztomczak · 2017-08-05T08:58:36Z

examples/pysdl2.py

+"""
+ Simple SDL2 / cefpython3 example.
+
+ Only handles mouse events but could be extended to handle others.


This comment is no more valid:

Only handles mouse events but could be extended to handle others.

cztomczak · 2018-01-08T16:29:32Z

I have tested on Ubuntu 14.04 and it works fine. I will merge it now and test on Windows later, and make some code changes as well. Thanks.

cztomczak · 2018-01-08T16:54:26Z

Further changes in commit ddd6264.

cztomczak · 2018-02-20T13:14:09Z

I have tested pysdl2 example on Windows and it works very slow, not usable. See this comment:
#324 (comment)

neilmunday · 2018-02-20T18:14:56Z

Hmmm... I've never tried PySDL2 on Windows. Perhaps adding a call to SDL_Delay at the end of the rendering loop might do the trick?

cztomczak · 2018-02-20T19:04:32Z

@neilmunday Please let's continue discussion in Issue #324. See this comment with new details: #324 (comment)

neilmunday added 2 commits July 30, 2017 22:37

Created pysdl2 example for issue cztomczak#324.

74106b8

Tidied up imports for pysdl2 example.

db2d000

cztomczak requested changes Jul 31, 2017

View reviewed changes

Updated coding style and comments.

3df4e74

cztomczak requested changes Aug 1, 2017

View reviewed changes

More styling fixes.

a782987

Added keyboard support

152edfb

cztomczak requested changes Aug 4, 2017

View reviewed changes

Fixed mouse motion handling and added wheel event handling.

1bc4d7f

cztomczak requested changes Aug 5, 2017

View reviewed changes

pysdl2 example complete.

7746edb

cztomczak reviewed Aug 5, 2017

View reviewed changes

cztomczak approved these changes Aug 5, 2017

View reviewed changes

Updated comments

279442d

neilmunday closed this Aug 7, 2017

neilmunday reopened this Aug 7, 2017

cztomczak merged commit acaf76b into cztomczak:master Jan 8, 2018

cztomczak mentioned this pull request Jan 8, 2018

PySDL2 example. Partially done. #324

Closed


		sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO)

		window = sdl2.video.SDL_CreateWindow('cefpython3 SDL2 Demo', sdl2.video.SDL_WINDOWPOS_UNDEFINED, sdl2.video.SDL_WINDOWPOS_UNDEFINED, width, height, 0)

pysdl2 example for issue #324 #379

pysdl2 example for issue #324 #379

Conversation

neilmunday commented Jul 30, 2017 • edited Loading

cztomczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neilmunday commented Jul 31, 2017

neilmunday commented Aug 1, 2017

cztomczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neilmunday commented Aug 2, 2017 • edited Loading

cztomczak commented Aug 2, 2017 • edited Loading

neilmunday commented Aug 3, 2017 • edited Loading

cztomczak commented Aug 3, 2017

neilmunday commented Aug 3, 2017

cztomczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neilmunday Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neilmunday Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neilmunday commented Aug 4, 2017

cztomczak commented Aug 4, 2017 • edited Loading

neilmunday commented Aug 4, 2017

cztomczak commented Aug 5, 2017

cztomczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cztomczak commented Aug 5, 2017

neilmunday commented Aug 5, 2017

cztomczak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cztomczak commented Jan 8, 2018

cztomczak commented Jan 8, 2018

cztomczak commented Feb 20, 2018

neilmunday commented Feb 20, 2018

cztomczak commented Feb 20, 2018

neilmunday commented Jul 30, 2017 •

edited

Loading

neilmunday commented Aug 2, 2017 •

edited

Loading

cztomczak commented Aug 2, 2017 •

edited

Loading

neilmunday commented Aug 3, 2017 •

edited

Loading

neilmunday Aug 4, 2017 •

edited

Loading

neilmunday Aug 4, 2017 •

edited

Loading

cztomczak commented Aug 4, 2017 •

edited

Loading