Skip to content
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.

std::min requires #include <algorithm> #3

Closed
xzcvczx opened this issue Sep 22, 2016 · 20 comments
Closed

std::min requires #include <algorithm> #3

xzcvczx opened this issue Sep 22, 2016 · 20 comments

Comments

@xzcvczx
Copy link
Contributor

xzcvczx commented Sep 22, 2016

I am not sure if you care about osx at this early stage but i tried to build it and had some errors the first of which

src/fastuidraw/painter/painter_brush.cpp:268:36: error: no member named 'min' in
namespace 'std'; did you mean 'fmin'?
static_cast(std::min(im->slack() + 1,
^~~~~~~~
fmin

This can be resolved by including in painter_brush.cpp (or in painter_brush.hpp)

@krogueintel
Copy link
Contributor

krogueintel commented Sep 24, 2016

I will post a fix shortly. Looks like on Linux and MinGW, the std::min got in there from includes. Was there any other build errors? I'd like to fix them all to get Mac OS-X atleast building successfully.

@xzcvczx
Copy link
Contributor Author

xzcvczx commented Sep 24, 2016

On OSX most people would likely install boost using brew. This means that the include/lib paths are inside of /usr/local. Currently i have hacked the makefiles to add a $(CFLAGS) and $(LDFLAGS) options able to be given as env variables. The other option would be to have something similar set up in the Makefile.settings.mk file. (The next most likely method of installation for boost would likely be macports which i think uses /opt)

-lboost_thread is not available only -lboost_thread-mt i am not sure if this is something related to the version of boost or a boost build option, So this may just require a DARWIN_BUILD similar to MINGW_BUILD with it linking the mt options rather than the single threaded versions.

The other issue i have run into is -soname. I don't have a linux system set up so is this option required if the -soname does not differ from the -o? This would also require DARWIN_BUILD as .dylib is the standard extension on OSX rather than .so.

I can make most of these changes but i would like some input on how you would like things done. The main reason this initial one was just a single small issue was because i wasn't sure whether you were accepting portability and other toolchain/platforms as of yet

After changing ngl/rules.mk trying to build make libFastUIDrawGL i also run into

src/fastuidraw/gl_backend/gl_binding.cpp:106:14: error: no member named
      'get_function_ptr_glGetError' in namespace 'fastuidraw::gl_binding'
  if(fptr == FASTUIDRAWglfunctionPointer(glGetError))
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
inc/fastuidraw/gl_backend/ngl_gl.hpp:30:67: note: expanded from macro
      'FASTUIDRAWglfunctionPointer'
#define FASTUIDRAWglfunctionPointer(name) fastuidraw::gl_binding::get_fu...
                                          ~~~~~~~~~~~~~~~~~~~~~~~~^
<scratch space>:6:1: note: expanded from here
get_function_ptr_glGetError
^
src/fastuidraw/gl_backend/gl_binding.cpp:167:35: error: no member named
      'get_function_ptr_glGetError' in namespace 'fastuidraw::gl_binding'
  ...&& fptr != FASTUIDRAWglfunctionPointer(glGetError) && logger())
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
inc/fastuidraw/gl_backend/ngl_gl.hpp:30:67: note: expanded from macro
      'FASTUIDRAWglfunctionPointer'
#define FASTUIDRAWglfunctionPointer(name) fastuidraw::gl_binding::get_fu...
                                          ~~~~~~~~~~~~~~~~~~~~~~~~^
<scratch space>:6:1: note: expanded from here
get_function_ptr_glGetError
^

the rules.mk file will require having OpenGL/gl3.h in place of gl/glcorearb.h but again that would come back to a DARWIN_BUILD alternative to MINGW_BUILD.

@krogueintel
Copy link
Contributor

Let's do this: make a repo on github where you get it to build on OS-X, I monitor and try to merge the fixes you make and also expand the Readme for OS-X from your efforts.

I also thank you for doing this and I am really happy to merge in fixes you make once you get a successful build

@krogueintel
Copy link
Contributor

krogueintel commented Sep 24, 2016

On the GL errors it looks like the stuff I use to generate the NGL stuff is not getting glGetError which means that likely the hacks that generate NGL are failing. Sighs

Edit: oh you noted that the file had to change to having OpenGL/gl3.h instead of GL/glcorearb.h. So the error is not from the hack that is NGL generation, but just from incorrect input file for OS-X. See next comment please.

@krogueintel
Copy link
Contributor

I've made a minor tweak to Makefile.settings.pre to have a stub for DARWIN_BUILD. If you have the time, can you make the additions to Makefile.base.lib.mk and Makefile.gl_backend.lib.mk for the correct command to build the shared library? Also I have added Makefile.gl_backend.settings.mk for the values of the GL header path and values.. can you also get those happy for Darwin?

@pinealservo
Copy link

I managed to get it to build on OS X, including the NGL generation stuff. I did a couple of dirty hacks, but they shouldn't be too hard to clean up. I'll fork and put my modifications there for you to check out sometime in the next couple of days. Some of the demos run, but others hang after startup for reasons I haven't determined yet.

@krogueintel
Copy link
Contributor

Excellent! I look forward to it.

Can you name what demos hang? The demo painter-test must wok for any of the painter-* demos to even have a small chance to work. That demo just assembles to uber-vert and frag shaders and tries to compile and link them. If that hangs then the main interface, Painter, won't work. If the shaders fail to compile or link, the demo generates bad_shader* and bad_program* files to dump the errors.

@pinealservo
Copy link

Currently the glyph, gradient, and image tests seem to work, although the font glyph rendering seems to have some issues. Do you have any screenshots of what you expect it to look like?

The painter-test demo links everything successfully, but then prints this:

Failed to find uniform "fastuidraw_glyphTexelStoreUINT" in program 7 for initialization
Failed to find uniform "fastuidraw_glyphGeometryDataStore" in program 7 for initialization

All the painter demos print those same lines right before the UI hangs.

@pinealservo
Copy link

Here are the changes I made to get it to build: https://github.com/01org/fastuidraw/compare/master...pinealservo:darwin_build?expand=1

I'm not exactly sure what the things I commented out in the last few sections of the diff are meant to do, but there were missing symbols at link time before I did it.

@krogueintel
Copy link
Contributor

Firstly, THAKYOU very, very much for doing this. However, the current patch needs additional work before I can merge it. Here we go:

NGL-stuff:

The minor issue is the change of name from GL_RAW_HEADER_FILES to GL_INC_HEADER_FILES.
(files Makefile.gl_backend_settings.mk and src/fastuidraw/gl_backend/ngl/Rules.mk). The name change omits the necessary change to platforms off of Darwin.

src/fastuidraw/gl_backend/private/buffer_object_gl.hpp:

That change looks awfully fishy. In GL when asking GL what buffer is bound to a binding point GL_FOO_BUFFER, the value to feed glGetInteger is GL_FOO_BUFFER_BINDING; If the thing does not build without that change then the enumeration is missing from OpenGL/gl3.h which is really bad. A simple fix is to have a check if the enumerations are defined and if not #define them to the correct value.

src/fastuidraw/gl_backend/private/texture_gl.cpp:

Was this to fix a compile error or link error? If a compile error then that just means the function was not defined in OpenGL/gl3.h which makes sense. IF a link error, against glCopyImageSubData, then there is a far deeper issue. The NGL system is supposed to be so that GL function pointers are fetched at run time via the function passed to fastuidraw::gl_binding::get_proc_function().... if that is not happening then the NGL file generation is somehow failing.

src/fastuidraw/gl_backend/private/texture_view.cpp

Same story as texture_gl.cpp.

On the subject of NGL. what I should do is that the function/macro names from NGL should have a different prefix from gl, like maybe ngl so that the thing never pulls in gl functions directly from the system headers. This is a giant hassle to do, but it looks like I should do it to help Darwin come along.

On the subject of demos, if you add to the command line of any of the demos when built for DEBUG (release does not have call backs on GL calls)

log_gl_commands filename

ALL GL calls of the application will be logged to filename if the NGL system is working. IF it is not, then the file is empty. Test it out and let me know what happens.

@krogueintel
Copy link
Contributor

On the subject of demos, press 'd' to cycle though drawing a glyph different ways. If they all look bad then something is going very wrong. If just the curve analytic one is bad, then it looks like the GLSL compiler on Apple's platform is buggy and I will need to find workarounds for it. Though, without my own OS-X device on which to test that is quite tricky.

@pinealservo
Copy link

Thanks for looking it over. I wasn't really expecting it to be ready for merge, as I wasn't thinking very hard about contributing the changes while I was making them and I haven't had a lot of time to spend on it since. But I do think a few parts bear futher discussion before I try to resolve them:

NGL-stuff:

The minor issue is the change of name from GL_RAW_HEADER_FILES to GL_INC_HEADER_FILES. (files Makefile.gl_backend_settings.mk and src/fastuidraw/gl_backend/ngl/Rules.mk). The name change omits the necessary change to platforms off of Darwin.

Maybe I need to reverse the sense of the RAW vs. INC so that it doesn't break other things, but my code right now has both, and they're both needed. IIRC, one of them ends up used in compiler invocations and the other in the NGL code gen process. The odd place in which OS X puts the headers means that the same format doesn't work for both. I'll need to go through it again to give more details as to why, though.

src/fastuidraw/gl_backend/private/buffer_object_gl.hpp:

That change looks awfully fishy. In GL when asking GL what buffer is bound to a binding point GL_FOO_BUFFER, the value to feed glGetInteger is GL_FOO_BUFFER_BINDING; If the thing does not build without that change then the enumeration is missing from OpenGL/gl3.h which is really bad. A simple fix is to have a check if the enumerations are defined and if not #define them to the correct value.

It does look super fishy, and I didn't really like it, but I did verify that both macros actually expand to the same number in the Khronos headers that have both. My knowledge of OpenGL is really superficial right now, so I was not sure what semantic distinction was being made with the names, and looking at API documentation superficially didn't clear things up. I will look further into it next time I get a chance to work on it.

Regarding the stuff removed from texture_gl.cpp and texture_view.cpp; apparently they're compiler errors regarding missing symbols rather than link errors, which does make more sense. I'm guessing those APIs are too new to be supported, https://www.opengl.org/sdk/docs/man/html/glCopyImageSubData.xhtml says this one was introduced in OpenGL 4.3 while the GL_VERSION macros in OpenGL/gl3.h only go up to GL_VERSION_4_1_1.

As for the font rendering in the demos: The glyph shapes looked correct and were visually okay at large sizes, but the antialiasing seemed to be very off. Small fonts are jaggy no matter what, but changing the antialiasing amount had a strange effect. It tended to look best at 0 or 1 but get progressively worse from there. I'll try to get some more details for you along with some images at some point.

@krogueintel
Copy link
Contributor

krogueintel commented Oct 8, 2016

That the values expand to the same for those buffer binding points.. but for other binding points they are not the same. Joy. However, I strongly prefer to keep the _BINDING macro for the argument to context_get<> as in master, unless those are missing from the OS-X system.

For the functions, not in OS-X's OpenGL/gl3.h, I think I can make some macro checks to check if the functions exist.

On the subject of the font rendering, the GPU font rendering needs enough pixels to look right (thus low PPI it looks off). That the anti-aliasing looks off likely means I need to spend more time on glyph rendering and anti-aliasing it better.

@pinealservo
Copy link

Yes, the _BINDING ones are missing on OS X OpenGL/gl3.h, or else I wouldn't have changed to the ones without _BINDING. Maybe they can be conditionally added somewhere; I was being a bit sloppy at that point as I wasn't certain I'd even be able to get it to work.

I collected a bunch of screenshots of the various antialiasing settings on the curvepair glyphs at both the default zoom level and a much higher zoom level. Things look pretty nice at high zoom and low-aa settings, but high-aa settings start to introduce more jagginess. I left the screenshots on my work Linux machine, but I could send them on Monday. I've seen this behavior on all the systems I've built and run it on, though, so I would suspect it's working the same way as on yours.

@krogueintel
Copy link
Contributor

I've made a change to AA-mode ordering on glyph-test to be not as insane.

Roughly speaking AA-mode's 0 and 1 are supposed to be good (these are the one used by Painter). Mode 2 should look awful under minification, mode 3 has good quality on some zoom ranges and mode 4 represents no anti-aliasing.

In truth I am not 100% happy with mode 0 and 1, but if they are mostly acceptable then I am going to put it on the back burner to improve font rendering (this is important though!).

@krogueintel
Copy link
Contributor

krogueintel commented Oct 10, 2016

One more thing, if the painter demos hang the system, this is BEYOND serious. If painter cannot run, then almost all of FastUIDraw cannot run. Do you know where the painter demos make the system hang? My only hunch at this point as to what is going on is that the uber-shader is too much for OS-X's implementation of GL and when it finally tries to really generate (or use) the machine code to feed the GPU, the system hangs. If that is the case, I am going to need an OS-X device on which to test, diagnose and hopefully fix the issue.

Or cry uncle and write a Metal backend.

@pinealservo
Copy link

The explanation of the AA settings makes the behavior a lot less strange. Text rendering is good above a certain size, but could definitely use some work at normal "body text" sizes, especially with long slants like in "W".

I haven't tried any fancy debugging techniques on the hang yet; my only clue to offer about the hang is that it prints these two lines:

Failed to find uniform "fastuidraw_glyphTexelStoreUINT" in program 7 for initialization
Failed to find uniform "fastuidraw_glyphGeometryDataStore" in program 7 for initialization

I ran the painter-test-GL-release on the OS X system as well as on a working Linux system; the output glsl files look pretty much the same when I search for those uniform names. I'm not sure how I'd identify which bits correspond to "program 7". The output does say that linking was successful, so it's probably occurring as part of the load/init process for the shader programs, or maybe a bug in the linker part of the drivers where it didn't write the correct linker metadata? I don't really know anything about the linking and loading aspects of glsl compilers.

If you have any suggestions for further diagnosis I'd be happy to try them.

@krogueintel
Copy link
Contributor

krogueintel commented Oct 14, 2016

Hi,

I will take a look into those two messages. Can you run ./painter-test-GL-debug print_gl_info true
and send me the output and in addition the six generated files painter.*.glsl ?

The message failed to find uniform is coming from that it is expecting that some uniform FOO is active in the shader, but it is not. Those uniforms fastuidraw_glyphTexelStoreUINT and fastuidraw_glyphGeometryDataStore are the sampler/samplerBuffer uniforms to access the glyph stuff, so them missing is very serious.. unless it is for the uber-shader that has discard (that shader does not do any glyph rendering).

Try running with

"separate_program_for_discard false"

this option sets it so that there is only one uber-shader. If the output of missing uniforms still comes, then we have a serious issue.

I've also made a commit to try to use glGetUniformLocation if gl_program::uniform_location() fails. Please try the latest version.

@krogueintel
Copy link
Contributor

I've (finally) added your fixes for OS-X dressed up.. by using #ifdef APPLE

@krogueintel
Copy link
Contributor

Sighs.. Apple has listed OpenGL as deprecated on macOS. As such, getting FastUIDraw to work on MacOS with the GL back end is now dead. I still want have FastUIDraw work there, but it will need to be done by a Metal back end.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants