FLTK logo

STR #3477

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | SVN ⇄ GIT ]

STR #3477

Application:FLTK Library
Status:1 - Closed w/Resolution
Priority:1 - Request for Enhancement, e.g. asking for a feature
Scope:3 - Applies to all machines and operating systems
Subsystem:Core Library
Summary:add oxy scheme
Version:1.4-feature
Created By:lm
Assigned To:AlbrechtS
Fix Version:1.4.0
Fix Commit:b1ba37c5ba1df543baa87d328805af34da4bd2b1
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size  
 
#1 lm
11:35 Jun 14, 2018
patchscheme2.diff
33k
 
 
#2 AlbrechtS
02:27 Jun 15, 2018
oxy_v3.diff
32k
 
 
#3 AlbrechtS
03:15 Jun 15, 2018
arrows_dont_scale.png
20k
 
 
#4 AlbrechtS
06:18 Jun 15, 2018
oxy_v4.diff
35k
 
 
#5 AlbrechtS
08:30 Jun 15, 2018
arrows_scale_poc.png
48k
 
 
#6 AlbrechtS
11:58 Jun 16, 2018
oxy_v5.diff
39k
 
 
#7 AlbrechtS
12:17 Jun 16, 2018
oxy_v5.png
63k
 
 
#8 AlbrechtS
12:25 Jun 16, 2018
oxy_vs_gleam.png
55k
 
     

Trouble Report Comments:


Name/Time/Date Text  
 
#1 lm
11:35 Jun 14, 2018
I'd like to try once again to get Dmitrij K's oxy theme added officially to FLTK (so I don't have to keep patching it when I build it).  I've put together a patch that's a subset of the oxy theme using the latest version of FLTK I could download (fltk-1.4.x-r12938).  I've tried to make the modifications look as much like the other schemes as possible so the changes will hopefully be compatible with the FLTK theme conventions.  I also noticed some inconsistencies on how FLTK was checking which theme was active, so I made a few minor modifications to try to fix that as well.  I am attaching a file with the patches.  I hope we'll finally be able to get some form of the oxy theme into FLTK 1.4.x.  Thanks.  
 
#2 AlbrechtS
02:27 Jun 15, 2018
Thanks for this new patch. Looks good, great job!

I modified the patch so it is more CMP compliant and uploaded a new version: oxy_v3.diff.

This includes the following changes (this list may not be complete though):

1) add missing file to CMakeLists.txt
2) reformat src/fl_oxy.cxx with clang-format (sorry, big diffs)
3) replace '///' comments with '//' to avoid confusing doxygen
4) add "oxy" scheme to unittests demo program
5) revert making Fl::is_scheme() non-inline

This modified patch appears to me to be acceptable for FLTK or at least very close. I did not yet check all the changes to widgets, so there may well be some issues, but I'll do that soon (when time permits).

So far I believe we have something that can be used to do some final cleanup and commit eventually.

See follow-up...
 
 
#3 AlbrechtS
02:44 Jun 15, 2018
@Laura: As you noticed I reverted your changes that make Fl::is_scheme() a non-inline method. Please elaborate why you think this should be changed. Thank you.

My reasoning is:

(1) this method should really be inline
(2) it can use strcmp() to be as fast as possible.

Using strcmp() would likely be faster than fl_ascii_strcasecmp(name,scheme_). The FLTK docs say explicitly that only lowercase strings are allowed as scheme names, particularly for comparison, hence we can avoid to deal with case insensitive comparison. The scheme name is always stored in lowercase letters, and only characters in the ASCII range are allowed.

I don't see an issue with strcmp() as such, even Microsoft seems to support it w/o warnings (although I didn't test it with VS). Their docs say:

"The strcmp functions [...] are not affected by locale."
https://msdn.microsoft.com/en-us/library/e0z9k731.aspx
 
 
#4 AlbrechtS
02:48 Jun 15, 2018
@Dmitrij: Two questions:

(1) Would it be okay for you to include this reduced version of your scheme in FLTK?

(2) Would it also be okay to add the usual FLTK copyright disclaimer with a note that the scheme was contributed by you?
 
 
#5 AlbrechtS
03:15 Jun 15, 2018
Further testing showed an issues with arrow drawing that doesn't scale well (for instance in scrollbars). AFAICT the arrow drawing function is called with x/y coordinates but w/o width and height information. This should be fixed by giving it a bounding box.

See image http://www.fltk.org/strfiles/3477/arrows_dont_scale.png

In general *any* widget element that can be drawn differently in schemes (i.e. dependent on the active scheme) should be given a bounding box to draw into. If necessary, this bounding box could even be clipped to avoid drawing outside of the box. Note: This is something I'm planning for the future scheme development but this is not necessary here.
 
 
#6 AlbrechtS
03:45 Jun 15, 2018
Hmm, regarding copyright: I read STR #2675, comment #24 by Dmitrij that we are allowed to use the FLTK copyright for Dmitrij's contributions. If there is no other response I'll change the copyright accordingly.  
 
#7 AlbrechtS
06:17 Jun 15, 2018
Next iteration: adding some missing parts (fluid, documentation, unittests (again)).

See file:
 
 
#8 AlbrechtS
06:18 Jun 15, 2018
... oxy_v4.diff  
 
#9 AlbrechtS
08:30 Jun 15, 2018
Just to let you know: I'm working on the scaling issue with bounding box. As a proof of concept you may take a look at the attached image arrows_scale_poc.png, but this image is heavily scaled up so it is only a proof that the arrows *can* scale down, but not how it will look when ready.

I'm going to take a break now, will continue probably tomorrow...
 
 
#10 AlbrechtS
11:58 Jun 16, 2018
The new patch oxy_v5.diff fixes the scaling issue (not yet perfect, but anyway...) and uses an extended fl_draw_arrow() function with a bounding box (Fl_Rect), a type parameter (Fl_Arrow_Type) and an orientation parameter (Fl_Orientation). Fl_Arrow_Type and Fl_Orientation are new enums.

This is experimental, but the reasoning is to use this arrow drawing function for all FLTK schemes in the future. I believe it's pretty good so far, but there may be changes of function parameters and enum values in the future.

This patch extends the oxy scheme for Fl_Counter and replaces the (IMHO) ugly FLTK arrow symbols with the "oxy" style arrows.

Please test. Comments welcome.
 
 
#11 AlbrechtS
12:17 Jun 16, 2018
Attached file oxy_v5.png shows a screenshot of current development (oxy_v5.diff).

Differences to Dmitrij's original patch:

- no size changes of widget elements
- Fl_Choice has a double down arrow (previously a single down arrow)
- Fl_Counter uses "oxy" arrows instead of FLTK arrow symbols
- arrow drawing has been rewritten (may be discussed further)

@Dmitrij and others: it is *very* easy now to change the appearance of Fl_Choice so it uses a single arrow as before. It's only another enum value in the fl_draw_arrow() call.

The rewritten arrow drawing uses a transformation matrix to be able to rotate arrows in 90° angles. Currently the "shadow effect" may not be entirely "correct" WRT the old implementation. This can be changed with a little effort, but for now I believe it's okay (hard to tell the difference). Again, comments welcome.

I see this as a good starting point for a commit to get the "oxy" scheme into the FLTK library.
 
 
#12 AlbrechtS
12:25 Jun 16, 2018
FYI: see attached file oxy_vs_gleam.png  
 
#13 lm
04:49 Jun 18, 2018
On Fri, Jun 15, 2018 at 5:44 AM, Albrecht Schlosser <AlbrechtS.fltk@online.de> wrote:
> @Laura: As you noticed I reverted your changes that make Fl::is_scheme() a
> non-inline method. Please elaborate why you think this should be changed.

I was thinking at the time that moving the function in the file and saying it was inline would probably work almost as well as having it in the header.  The compiler is going to decided if it's inline or not anyway.

> (1) this method should really be inline
> (2) it can use strcmp() to be as fast as possible.

> Using strcmp() would likely be faster than
> fl_ascii_strcasecmp(name,scheme_).

I agree, strcmp is faster and should be available with practically all C compilers.  The original code had some of the calls to strcmp and some of the calls to fl_ascii_strcasecmp.  Was simply trying to make it more consistent.  If fl_ascii_strcasecmp is not used, there's no need for the function to be moved to a file.  It can be left in the header.

I'll try e-mailing Dmitrij.  It's been a couple of years since I've heard from him by e-mail.

Also, hoping he has some suggestions on arrow changes.  I pretty much used his drawing routines as is and just tried to make things fit in better with how FLTK expected to interface with the theme.  I like the way the arrow code looks with the scheme.  Maybe we can come up with some code that's more vector-like and thus might scale better.
 
 
#14 AlbrechtS
11:10 Dec 06, 2021
Status report: I've been working on the addition of the new scheme but I got stuck. However, I'm still maintaining a local branch to include this scheme in the future.

I can't tell today if this has a chance to make it into FLTK 1.4.0 but it's not forgotten and will be done at some time, hopefully soon.

FTR:

The original patch for this scheme was provided by 'kdiman' in STR #2675 but he doesn't seem to be around anymore. I tried to contact him, but w/o success.

I'm closing STR #2675 which is now obsolete. For further reference please see:
https://www.fltk.org/str.php?L2675
 
 
#15 AlbrechtS
05:35 Nov 26, 2022
Fixed in Git repository.

Finally this new scheme found its way into FLTK 1.4.0 (current git)
in commit b1ba37c5ba1df543baa87d328805af34da4bd2b1.

Thanks for the original patch to Dmitrij (kdiman) and for the updated patch and helpful discussion to Laura (lm).

Closing this STR now.
 
     

Return to Bugs & Features ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2024 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.