Page 1 of 2 12 LastLast
Results 1 to 20 of 21

Thread: Various fixes and improvements

  1. #1

    Default Various fixes and improvements

    Hi,

    I'll post here various small fixes and changes all over the ClanLib. Here is the detailed list of changes:

    • Fixed CL_OpenGLProgramObjectProvider::get_uniform_locati on() that didn't handle array subscript expressions like "array[13]"
    • Fixed CL_OpenGLProgramObjectProvider::get_attribute_coun t() that caused stack overflow
    • Fixed copy & paste error in CL_ProgramObject::detach()
    • Added missing CL_OcclusionQuery::get_provider() and CL_ShaderObject::get_provider() functions
    • Added complementary CL_IODevice::write_string_nul() and CL_IODevice::write_string_text() functions
    • Added CL_VirtualFileSystem::get_path() function
    • Modified CL_ShaderObject:: operator == to compare implementations, not OpenGL handles
    • Modified CL_Win32Window::modify_window() function to apply new window title and topmost flag
    Attached Files Attached Files

  2. #2

    Default

    Bump......

  3. #3
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    Many thanks.

    1) Fixed copy & paste error in CL_ProgramObject::detach()

    Well spotted

    2) Added missing CL_OcclusionQuery::get_provider() and CL_ShaderObject::get_provider() functions

    Nice

    3) Added complementary CL_IODevice::write_string_nul() and CL_IODevice::write_string_text() functions

    Nice

    4) Added CL_VirtualFileSystem::get_path() function

    Nice

    5) Modified CL_ShaderObject:: operator == to compare implementations, not OpenGL handles

    Yeah, for ClanLib API consistancy, I think that you are right

    6) Modified CL_Win32Window::modify_window() function to apply new window title and topmost flag

    Okay. (But as far as i know, the function is never called from the API)

    7) Fixed CL_OpenGLProgramObjectProvider::get_uniform_locati on() that didn't handle array subscript expressions like "array[13]"

    I see you have (indirectly) removed the cache. The cache was added for increased performance. It is preferable (if possible) to retain the cache (in some form). If not, then okay.

    8) Fixed CL_OpenGLProgramObjectProvider::get_attribute_coun t() that caused stack overflow

    I think would be better to use the cache. The stack overflow is caused by:
    Code:
     int CL_OpenGLProgramObjectProvider::get_attribute_count() const
     {
         ...
         if (cached_attribs.empty())
             fetch_attributes();
         ...
     }
     void CL_OpenGLProgramObjectProvider::fetch_attributes() const
     {
         if (!cached_attribs.empty())
             return;
         ...
         int count = get_attribute_count();
         ...
     }
    The get_attribute_count() call in fetch_attributes() should be replaced with

    "clGetProgramiv(handle, CL_ACTIVE_ATTRIBUTES, &count);"

    (This is my opinion, other developers may have another opinion)

  4. #4

    Default

    I see you have (indirectly) removed the cache. The cache was added for increased performance. It is preferable (if possible) to retain the cache (in some form). If not, then okay.
    Well, I'll think about it. The cache is a good thing, but unfortunately it doesn't work with individual array elements. For now direct OpenGL calls perform more slowly than the cache search, but I think it's because of redundant state tracker construction. I'll check it later in my spare time.

    The get_attribute_count() call in fetch_attributes() should be replaced with
    "clGetProgramiv(handle, CL_ACTIVE_ATTRIBUTES, &count);"
    You're right, it is even better solution Okay, there seems to be no other issues, so I've attached an edited patch without CL_OpenGLProgramObjectProvider::get_uniform_locati on() stuff.
    Attached Files Attached Files

  5. #5
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    Quote Originally Posted by toiffel View Post
    For now direct OpenGL calls perform more slowly than the cache search, but I think it's because of redundant state tracker construction.
    You have a good point, the state tracker looks reduntant because we are only using clGetAttribLocation(handle...) etc. We are not changing the state of a program object.

    I don't know who added the cache, and why. Maybe it is not required if the state tracker is not used?

    Maybe the creator of the cache, did not think of this?

    (Note, CL_OpenGL::set_active(gc_provider) is still required - but it does not do anything if the gc is still active)
    Last edited by rombust; 12-08-2010 at 08:07 PM. Reason: spelling

  6. #6
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    Applied small_fixes2.patch to ClanLib 2.2 SVN (since it did not break API) and ClanLib 2.3 SVN.

    Many Thanks.

  7. #7

    Default

    You have a good point, the state tracker looks reduntant because we are only using clGetAttribLocation(handle...) etc.
    Exactly. Almost all OpenGL shader functions except glUniform* accept program handle as a first argument and don't require an active program object to be bound. Therefore most state trackers should be safely replaced with CL_OpenGL::set_active(gc_provider) call.

    Without a state tracker in CL_OpenGLProgramObjectProvider::get_uniform_locati on() the cache is not required too. However it's still useful for get_uniforms()/get_uniform_count() and get_attributes()/get_attribute_count() methods.

    The attached patch removes all redundant state trackers from CL_OpenGLProgramObjectProvider. Also I've added equality operators to CL_FrameBuffer, CL_ProgramObject and CL_RenderBuffer classes.
    Attached Files Attached Files

  8. #8

    Default

    ... Up ...

  9. #9
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    Whoops, missed that one.

    Applied to SVN (2.2 and 2.3)

    Many Thanks.

  10. #10

    Default

    Some more additions, primarily for linux:

    • Added new signal CL_DisplayWindow::sig_window_restored.
    • Implemented minimize/maximize/restore methods and signals for CL_X11Window using WM_STATE and _NET_WM_STATE properties. Tested on Gentoo x86_64/KDE and Ubuntu 10.10 x86/Gnome but should work with any modern window manager.
    • Removed unused title, allow_resize and layered members from CL_X11Window.
    • Added UCS-2 -> UTF-8 conversion for command-line arguments in Win32 clanapp.
    • Added CL_SharedGCData::add_texture member. Allows you to fill the texture cache with custom textures and then construct CL_Sprite objects from them (I've used it for loading textures in background).
    Attached Files Attached Files

  11. #11
    ClanLib Developer
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    554

    Default

    Thanks for the patch.

    I've applied it to the 2.2 and 2.3 branches.

  12. #12

    Default

    Currently I'm dealing with user profiles/settings manager which heavily uses CL_Directory. Unfortunately this class seems a bit outdated :wheelchair: so I've made some improvements to it

    • CL_Directory::create() can now create directories recursively. A new argument 'recursive' was added. By default it's set to false for 2.2 compability.
    • CL_Directory::remove() was cleaned up a lot.
    • CL_Directory::rename() was added.
    • CL_Directory::get_current() now uses GetCurrentDirectory() on Win32.
    • CL_Directory::get_appdata() and CL_Directory::get_local_appdata() were vastly simplified. Linux implementation now uses getpwuid() instead of $HOME environment variable.
    • CL_Directory::get_resourcedata() got a new argument 'data_dir_name' so you can specify the data directory other than default "Resources". This function is also implemented on Linux, although it's much better to obtain this path from the build system.

    This patch is for ClanLib 2.3 SVN only since it breaks some API.
    Attached Files Attached Files

  13. #13
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    Applied patch to SVN 2.3

    I tested it with Tests/Core/IOData on WIN32 ... It passed with the existing tests.

    (that was after I first modified the test application because it contained a bug)

    (I have not added any new tests to demonstrate the new functionality.)

  14. #14

    Default

    I always carefully test my patches on both Windows and Linux. They should work in most cases without any problem

    By the way I've searched through the entire source code for 'CL_Directory::' keyword and didn't find any occurences (except those in model.cpp and directory.cpp itself). So ClanLib actually doesn't make any use of it and a separate test will be welcomed.

    (Maybe I'll write this one someday).

  15. #15

    Default

    I've prepared another small patch for 2.3 SVN that fixes some minor issues with the API:
    • Added support for vertex attribute locations to CL_ProgramAttribute class
    • Added missed 'const' qualifier to some is_null() methods
    • Maximize button is now disabled when 'allow_resize' is false (see TODO)
    • CL_Texture constructor is changed to CL_Texture(gc, resource_id, resources, import_desc) (see TODO)
    Attached Files Attached Files

  16. #16
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    Committed. Many thanks

  17. #17

    Default

    A next patch that is mainly for 2.3 SVN. Please post your comments if it contains some unwanted functionality.
    • Fixed CL_Sprite::draw(gc, src_rect, dest_rect)
    • Added CL_Thread::kill() (existed earlier in ClanLib 1.0)
    • CreateThread() is replaced with _beginthreadex() in CL_Thread_Win32 (as recommended in microsoft docs)
    • Added methods for getting/setting float attributes to CL_DomElement
    • Added CL_Font_Freetype constructor for loading truetype fonts from the resource files
    • Some linux compilation fixes

    A few words about the font resources. Now you can construct CL_Font_Freetype font from a resource file using something like:
    HTML Code:
    <font name="ClanFont">
        <freetype file="myfont.ttf" height="14" average_width="0" anti_alias="true" subpixel="true"/>
    </font>
    Only 'file' and 'height' attributes are required, other attributes are optional.

    This patch is tested on 2.3 SVN but it can be partially applied to 2.2 SVN as well. I believe CL_Sprite, CL_Thread and CL_DomElement can be patched without any trouble.
    Attached Files Attached Files

  18. #18
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    All commited to ClanLib 2.3 SVN
    All except the 2.3 specific linux compile fixes commited to ClanLib 2.2 SVN

    Many thanks.

  19. #19

    Default

    By the way, I've noticed that you've modified CL_Directory::create() function (SVN rev. 5660).

    This function was designed so that when the 'recursive' argument is true, it creates all directories specified in the path (ignoring the possible errors). The function returns true if the last directory in the path was successfully created.

    But now the function fails to create directories specified with absolute paths. At first it tries to create the root directory (which is always exist) and returns immediately.

    So I think the old function behavior should be restored.

    Maybe the 'recursive' argument should be renamed to 'create_dirs_if_missing' (or something else) if it's unclear what the function does exactly?

  20. #20
    ClanLib Developer
    Join Date
    May 2007
    Posts
    1,824

    Default

    You are correct.

    The old behavior has been restored.

    (I added "bool result = true;" to stop GCC complaining about using an uninitialised variable when dir_name=="", recursive==true)

Similar Threads

  1. gcc fixes to subversion code
    By xrubio in forum Official ClanLib SDK Forums
    Replies: 2
    Last Post: 08-21-2009, 07:26 AM
  2. GUI Editor improvements
    By Harry in forum Official ClanLib SDK Forums
    Replies: 0
    Last Post: 08-14-2009, 05:10 AM
  3. Ideas for improvements
    By in forum Dungeon Scroll for PC and iPhone
    Replies: 4
    Last Post: 08-06-2009, 04:47 AM
  4. some improvements for smoother building
    By grgro in forum Official ClanLib SDK Forums
    Replies: 3
    Last Post: 11-13-2008, 09:49 AM
  5. Few improvements/problems
    By redink1 in forum Other RTsoft Games
    Replies: 1
    Last Post: 10-10-2002, 10:13 PM

Bookmarks

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •