[waffle] [PATCH] waffle: make gl_basic to work with nacl
Tapani Pälli
tapani.palli at intel.com
Mon Dec 29 00:00:34 PST 2014
On 12/23/2014 04:25 AM, Chad Versace wrote:
> On 12/21/2014 08:06 AM, Emil Velikov wrote:
>> On 17 December 2014 at 13:12, Tapani Pälli <tapani.palli at intel.com> wrote:
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> ---
>>> .gitignore | 2 ++
>>> examples/CMakeLists.txt | 30 ++++++++++++++++++++++++++++++
>>> examples/gl_basic.c | 17 +++++++++++++++++
>>> examples/index.html | 39 +++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 88 insertions(+)
>>> create mode 100644 examples/index.html
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index 6981cd7..a1161e3 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -34,6 +34,8 @@ Makefile
>>>
>>> /bin/gl_basic
>>> /bin/gl_basic_test
>>> +/bin/gl_basic_nacl.nexe
>>> +/bin/gl_basic_nacl.nmf
>>> /bin/simple-x11-egl
>>> /bin/wflinfo
>>> /doc/html/man/waffle.7.html
>>> diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt
>>> index 75e1c0b..cf75165 100644
>>> --- a/examples/CMakeLists.txt
>>> +++ b/examples/CMakeLists.txt
>>> @@ -17,6 +17,36 @@ if((waffle_on_linux) AND (waffle_has_x11_egl))
>>> endif()
>>>
>>> # ----------------------------------------------------------------------------
>>> +# Target: gl_basic_nacl (executable + JSON manifest file)
>>> +# ----------------------------------------------------------------------------
>>> +if (waffle_has_nacl)
>>> + add_executable(gl_basic_nacl.nexe gl_basic.c)
>>> + include_directories(${nacl_INCLUDE_DIRS})
>>> +
>>> + # this is done to get rid of -Werror=implicit-function-declaration
>>> + # which results in error when compiling against ppapi_simple
>>> + set(CMAKE_C_FLAGS "--std=c99")
>
>> Hmm I was under the impression that the we've already have it set in
>> the "top" cmake file ?
>> I.e. within cmake/Modules/WaffleDefineCompilerFlags.cmake
>
> Yes, but it looks like Tapani is using a quick hack to strip -Werror=implicit-function-declaration.
> His hack is to clobber any previously set value for CMAKE_C_FLAGS.
>
> Tapani, I think it's cleaner to change WaffleDefineCompilerFlags.cmake to
> not add the problematic flag if waffle_has_nacl==true.
Ah right, I did not remember to document this change. This is exactly to
get rid of 'implicit-function-declaration'. It's a shame that cmake does
not have a way to remove single flag for a target. I will add the change
to WaffleDefineCompilerFlags.cmake.
>>> +
>>> + target_link_libraries(gl_basic_nacl.nexe ${waffle_libname} ${nacl_LDFLAGS} -lppapi_simple -lnacl_io -lppapi_gles2)
>>> +
>> Please split it into separate lines, and double check for overlinking.
>
> Yes, please split.
OK, will do.
>> I would suspect that we don't need -lppapi_gles2, and maybe more.
Yep, gles2 lib can be removed. Others are required.
>>> + # create .nmf file that contains JSON manifest for the .nexe required by NaCl
>>> + add_custom_command(
>>> + OUTPUT gl_basic_nacl.nmf
>>> + COMMAND ${nacl_root}/tools/create_nmf.py -L${CMAKE_LIBRARY_OUTPUT_DIRECTORY} ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/gl_basic_nacl.nexe > ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/gl_basic_nacl.nmf
>>> + DEPENDS gl_basic_nacl.nexe
>>> + COMMENT "Create JSON manifest"
>>> + VERBATIM
>>> + )
>>> +
>>> + add_custom_target(CreateJSONManifest ALL
>>> + DEPENDS gl_basic_nacl.nmf)
>
> Please follow the convention elsewhere in Waffle and use unix-hacker-style or
> unix_hacker_style for target names, not CamelCase. Also, the name should
> be prefixed with "gl_basic".
OK, this happened because of the examples I found used CamelCasing. Will
fix.
>>> +
>>> + # install index.html that loads gl_basic_nacl.nmf
>>> + file(INSTALL index.html DESTINATION ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})
>>> +
>> If the following works, can we use it to be consistent with the rest of waffle ?
>>
>> install(
>> FILES index.html
>> DESTINATION ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}
>> )
>>
>
> The `file(INSTALL ...)` command is new to me, but if I understand the CMake
> docs correctly, it behaves differently than the `install(FILES ...)` command.
> The `install` command doesn't install the file until you run `make install`.
> The `file(INSTALL ...)` command installs the file immediately (but, um,
> I'm unsure exactly when "immediately" is).
>
> If I'm reading the patch right, index.html needs to be in the correct location
> during build time, not at install time.
>
> NaCl is still really new to me. So I don't know what the best thing to do is
> with the auxillary json and manifest files. But I am sure that putting a file
> named 'index.html' into $WAFFLE_BUILD_DIR/bin is the wrong thing to do, because
> that precludes the possibility of adding additional NaCl apps to the build
> system due to filename conflicts.
The intention was to have gl_basic_nacl.nexe, gl_basic_nacl.nmf and
index.html as output of the build in a single directory so that user can
pick them up from there. I'm not sure if bin is correct place either. My
suspicion is that any user would anyway copy them to where web server is
serving it's content, not to /usr/bin or such. Any advice appreciated
here, should I create a new output directory?
>>> +
>>> +# ----------------------------------------------------------------------------
>>> # Target: gl_basic (executable)
>>> # ----------------------------------------------------------------------------
>>>
>>> diff --git a/examples/gl_basic.c b/examples/gl_basic.c
>>> index fb62d52..371e423 100644
>>> --- a/examples/gl_basic.c
>>> +++ b/examples/gl_basic.c
>>> @@ -508,8 +508,16 @@ removeXcodeArgs(int *argc, char **argv)
>>>
>>> #endif // __APPLE__
>>>
>>> +#ifdef __native_client__
>>> +#include "ppapi_simple/ps_main.h"
>>> +int basic_test_main(int argc, char **argv);
>>> +PPAPI_SIMPLE_REGISTER_MAIN(basic_test_main)
>>> +int
>>> +basic_test_main(int argc, char **argv)
>>> +#else
>>> int
>>> main(int argc, char **argv)
>>> +#endif
>
> Bear with my NaCl-ignorant question.
>
> Why do you rename main to basic_test_main? They have the same signature.
> Is it because the NaCl runtime already defines a symbol named main?
Yes, this is because I use the ppapi_simple to minimize nacl specific
changes. It has main() defined.
>>> {
>>> bool ok;
>>> int i;
>>> @@ -530,9 +538,18 @@ main(int argc, char **argv)
>>> cocoa_init();
>>> #endif
>>>
>>> + #ifndef __native_client__
>>> ok = parse_args(argc, argv, &opts);
>>> if (!ok)
>>> exit(EXIT_FAILURE);
>>> + #else
>>> + // Fixed arguments for native client.
>>> + opts.context_api = WAFFLE_CONTEXT_OPENGL_ES2;
>>> + opts.platform = WAFFLE_PLATFORM_NACL;
>>> + opts.dl = WAFFLE_DL_OPENGL_ES2;
>>> + opts.context_profile = WAFFLE_NONE;
>>> + opts.context_version = -1;
>>> + #endif
>>>
>> Bikeshed:
>> Can we have consistent use of __native_client__. I.e.
>>
I can change this.
>> + #ifdef __native_client__
>> + // Fixed arguments for native client.
>> + opts.context_api = WAFFLE_CONTEXT_OPENGL_ES2;
>> + opts.platform = WAFFLE_PLATFORM_NACL;
>> + opts.dl = WAFFLE_DL_OPENGL_ES2;
>> + opts.context_profile = WAFFLE_NONE;
>> + opts.context_version = -1;
>> + #else
>> ok = parse_args(argc, argv, &opts);
>> if (!ok)
>> exit(EXIT_FAILURE);
>> + #endif
>
> In reply to your cover letter's question: I think #ifdefs are appropriate here.
OK cool
> _______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/waffle
>
More information about the waffle
mailing list