Open GoogleCodeExporter opened 9 years ago
BTW, this can be seen as a tie in to issue #91 as well, except that I'm just
pointing
out that you've essentially got that work done, but should just merge some of
the
scrollbar functionality into to the Slider, to make it more genericized for
what can
be done with it within GUIChan.
Original comment by irar...@gmail.com
on 3 Jul 2009 at 6:01
I'm not sure about a progress bar, but ScrollArea could definitely use two
Sliders.
However, ScrollArea is quite simple to override at the moment with its draw
methods.
With two Sliders it might not be as easy.
Original comment by olof.nae...@gmail.com
on 3 Jul 2009 at 8:00
Well, all I was saying was that a progress bar could easily be represented as
just
an override for the draw methods for a Slider, not that it can't be done more
efficiently (or that it should be included by default either. Personally, I
think
that it'd be better as its own class, and be left to programmers which are
using the
GUIChan library, since it isn't too difficult to do). I mainly mentioned it
just as
an example of what can already be done with them.
As for overriding draw methods in the ScrollArea, I suppose you could always
allow
the implementer to be able to supply their own slider, and if one is needed and
one
wasn't provided, create a new one for use within the ScrollArea using GUIChan's
inbuilt slider. Or it could just have an extra constructor just like the
LixtBox
does for its ListModel.
Likewise, you could move the drawing routines for a Slider to an external
rendering
class, which the slider could take. I know that GUIChan currently doesn't do
this,
and that doing so would currently break some GUIChan conventions for doing
things,
but this could potentially be nice for other widgets as well, like the TextBox,
for
instance. In the case of the TextBox, if users are allowed to have an external
rendering class, they could use it a lot like we're already able to with
ListBoxes,
and be able to supply their own interpretation on how to output the text within
it
(for instance, how to parse XML, HTML (which might be delegated further to
WebKit or
Gekko, for instance), or some other markup of their own invention). While users
can
simply create a new instance of a GUIChan class, and then make functions to
inherit
off of their own base class or even just create a few different rendering
functions
and then have which one is used controlled by some enumerated value, it's a bit
more
convenient and easier to maintain (in my opinion) if those functions are
delegated
to a helper rendering class instead. However, like the previous idea, this
would
also have an extra component to be passed in, whether it'd be optional or not.
Also, you could also move the scroll buttons to the slider class, then have
them
represented by a + and a - by default for drawing those buttons, but be able to
have
an option within the slider class to be able to show buttons or not. Just
thought
I'd bring that up as well, in case that hasn't crossed your mind yet.
In any case, it's nice to see that you can see some potential in this idea.
Original comment by irar...@gmail.com
on 6 Jul 2009 at 3:36
For the progress bar/slider issue, why not add a dedicated progress bar class
and
have the slider inherit from it instead of them being the same class?
Original comment by jaxad0...@gmail.com
on 6 Jul 2009 at 3:54
Jaxad, I never said that they would be the same class. If you had been reading
at
all, I was constantly mentioning overriding the draw classes, which is exactly
what
you're saying. However, in case it's of any benefit to you, ProgressBars can
already
be done through GUIChan if you follow these three simple steps:
1. Disable the slider.
2. Override the draw method (which should have yelled out inheritance here) to
draw a
rectangle for the full length of the widget, then draw a filled in rectangle to
the
slider marker's position.
3. (Optional) Update progress through the overriding class' logic() function,
or set
it in your program through the setValue() function if you don't update through
the logic.
4. (Optional) Set the slider to use Horizontal orientation.
What I think the real issue here is is whether Olof thinks that it's a good
idea to
include it in the base GUIChan library or not. To be honest, this doesn't really
bring a lot of value at all to the GUIChan library, and is better left as a user
implemented widget. However, I mentioned that idea for the sake of pointing out
the
versatility of the slider class as is, and as another counterpoint to how the
slider
could be utilized more generically.
Original comment by irar...@gmail.com
on 6 Jul 2009 at 4:14
A progress bar is perhaps the simplest widget there is, so I don't think an
implementation benefits at all from inheriting from Slider.
class ProgressBar: public Widget
{
public:
ProgressBar()
:mProgress(0)
{}
void setProgress(float progress)
{
mProgress = progress < 0.0f ? 0.0f : progress;
mProgress = progress > 100.f ? 100.0f : progress;
}
void draw(Graphics grapihcs)
{
graphics->setColor(getSelectionColor());
graphics->fillRectangle(0, 0, width * progress, height);
graphics->setColor(getForegroundColor());
graphics->drawRectangle(0, 0, width, height);
}
void logic{}
private:
float mProgress;
};
About ScrollAreas I now realize it's quite simple to just make a derived class
from
ScrollArea that sets new slider implementations to be use.
Original comment by olof.nae...@gmail.com
on 7 Jul 2009 at 7:15
Yup, agreed on that being the best implementation for a basic ProgressBar,
Olof. Like
I said: "Personally, I think that it'd be better as its own class, and be left
to
programmers which are using the GUIChan library, since it isn't too difficult
to do."
and "I was saying was that a progress bar could easily be represented as just an
override for the draw methods for a Slider, not that it can't be done more
efficiently". However, if we are talking about a simple override a Slider's
drawing
methods, though, it might be better be termed as a Meter, since that'd cover
more
types of behavior ranging from a simple slider skin to a bass meter for an
equalizer
if displayed vertically, but I'm not too sure on how many libraries actually
use that
terminology, hence why I stuck with just calling it a ProgressBar. In any case,
a
Meter would still just be a skin for a slider, and it's then just a visual
difference
when the behaviors underneath are essentially the same, and is definitely not
something that GUIChan should implement in that manner if at all.
However, if you think that a ProgressBar would be useful widget to have in the
base
GUIChan class, I'd say that that version is sufficient for most people's needs
(and
if it isn't, they can just extend it to be, so no big deal on that), and is
distinct
enough from sliders that it can justify a separate existence. I don't think
though
that the Slider should inherit from that class, especially when it's just as
much
work to override as it is to have a different function for setValue (and I also
think
that the name fits better as well for both of them, and that one shouldn't be
chosen
over the other when one would be slightly clumsy to the other).
But I do think that it would be good to point out some pros and cons with both
of the
methods which I was mentioning, since I'm commenting:
Passing Sliders directly to the ScrollArea class:
Pros:
1. Easy to code, and doesn't require a lot of work to do.
2. Allows for custom slider overrides to be easily used within the ScrollArea class.
3. If one of the bars is set to NULL, we can assume that means that the user
wants that specific scrollpolicy to be false (so we can actually get rid of
those two
options in the constructor).
Cons:
1. If the user wants to override the ScrollArea's scrollbars, they need to create
two Sliders, when they should only need to pass one. This could lead to
potential
mismatches.
2. While you could delete the Sliders when deleting the ScrollArea class, it
might not be too good of an idea, in case the user tied the Sliders to any other
actions, which means that we either need to make it clear that if you give a
slider
to the ScrollArea, it should only belong to the ScrollArea, or ignore the
memory leak.
Passing a renderer to the ScrollArea:
Pros:
1. We can ensure that both sliders keep the same visual appearance.
2. We can keep the Sliders self contained within the ScrollArea class, and
deleting them on ScrollArea deletion shouldn't be an issue.
Cons:
1. If an implementer makes some changes to the Slider class, they won't be able
to override them as easily (still doable in the constructors by deleting the
original
slider, then assigning their own, but that's kind of messy in a way).
2. While not necessarily harder to code than the previous one, it does require
more work to be done initially, not to mention that it doesn't follow the
current way
of doing things with GUIChan.
Given what I think you'd think on these issues, the renderer idea is probably
dead,
and you'll probably go with the two sliders being passed in instead, I think.
Which I
can understand easily enough, since it probably works best for TextBox-type
functionality anyways (when you get around to allowing use of bold, italicized,
or
other styled text within textboxes simultaneously). However, I still thought
that
it'd be nice to point out the strengths and weaknesses with both ideas anyways,
since
it does help a bit with identifying what could be some potential problems ahead
of
time, as well as to put forward a contrasting idea anyways even if I knew it
might
not make the cut anyways, if for no other reason than to point out some
potential
areas for improvement.
On a side note, though, is there any specific reason on why you use multiple
constructors when C++ has the option to specify a default value that the
compiler
should use if none is given? From my understanding, this has been in the
standards
for a while now (I think it was before the 1998 standard, which included
templates
and such if you're concerned about that. I think there's still some compilers
that
don't implement that standard fully, and if that's the case, I can see why you'd
avoid some language features that were implemented in that standard). Just in
case
you don't know what I'm referring to here, I'll use the ScrollArea as an example
here. In the header file, you'd do this:
ScrollArea::ScrollArea(Widget *content = NULL,
ScrollPolicy hPolicy = ShowAuto,
ScrollPolicy vPolicy = ShowAuto);
And then in the code file:
ScrollArea::ScrollArea(Widget *content,
ScrollPolicy hPolicy,
ScrollPolicy vPolicy)
{
mVScroll = 0;
mHScroll = 0;
mHPolicy = hPolicy;
mVPolicy = vPolicy;
mScrollbarWidth = 12;
mUpButtonPressed = false;
mDownButtonPressed = false;
mLeftButtonPressed = false;
mRightButtonPressed = false;
mUpButtonScrollAmount = 10;
mDownButtonScrollAmount = 10;
mLeftButtonScrollAmount = 10;
mRightButtonScrollAmount = 10;
mIsVerticalMarkerDragged = false;
mIsHorizontalMarkerDragged =false;
mOpaque = true;
setContent(content);
addMouseListener(this);
}
If you did this, you'd then have constructors for ScrollArea(),
ScrollArea(widget),
ScrollArea(widget, hPolicy, vPolicy), etc. and not have the same code repeated
because you wanted different orderings (but the user wouldn't be able to skip
ahead
to just putting in scroll policies, for instance, but they can't already do
that as
is). Mainly curious if you just didn't know that you could do that, or if you
actually don't do it because of specific compiler reasons.
Original comment by irar...@gmail.com
on 7 Jul 2009 at 6:09
There is no particular reason why we don't use default values. We could change
that
where it makes sense.
Supplying an object that draws the ScrollArea is a good idea, and people have
suggested it multiple times in the past. If such an object were to be used I
think it
should be used with the current draw method, so there is an option to use the
standard draw method.
About the ScrollArea, supplying two Sliders is probably the best idea for now.
And
no, Guichan should never delete things not allocated by Guichan.
Original comment by olof.nae...@gmail.com
on 8 Jul 2009 at 7:29
Ok, and that's basically what I was trying to point out there, since if they're
completely managed internally, it'd be safe to delete them, but so long as
there's
the possibility that they're external at all, then it comes down to whether to
leak
memory by default, delete it and then face the consequences, or I suppose
there's
also the possibility of using the typeid library (#include <typeid>, then use
an if
(typeid(*mHorizontalSlider) == typeid(Slider)) check ahead of time or so) to
ensure
that the slider is a GUIChan slider before deleting, then set it to null (so
that
there isn't an accidental memory corruption if it's accessed again), and then
the
implementer only needs to worry about their own slider implementations.
Ok, so if we were to separate out the drawing methods, would you want to do it
through inheriting from a separate Renderer class or so (which would then
contain a
draw method), much like they already inherit from the Widget class, or would you
rather have a rendering object in the code, like how the ListBox has a
ListModel?
Also, not trying to get too attached to that name, as DrawModel or another name
would
work just as well. I also suppose that it's possible to handle it both of those
ways
as well, or some other way as well, but it'd probably be better to stick with
just
one system rather than mixing them like that.
Original comment by irar...@gmail.com
on 8 Jul 2009 at 5:15
I think a visitor would be good that contains draw methods for all widgets.
Original comment by olof.nae...@gmail.com
on 8 Jul 2009 at 8:46
Just note that using a visitor, it is basically impossible to keep binary
compatibility. Whenever you'd add a new type of widget, you'd need to add a new
virtual function to the visitor, which would break any subclasses in user
applications. That said, there are so many problems with binary compatibility in
Guichan at the moment that this is probably irrelevant.
Still, a style might also want some control over margins and sizes used in the
Gui,
for example when a gcn::Button adjusts its size to its label, it could call
into the
style to get the distance between the text and the border. This is why QStyle
has a
bunch of virtual methods that take an enum value. The enums can be extended
without
breaking compatibility.
Original comment by b.lindeijer
on 9 Jul 2009 at 10:47
Original issue reported on code.google.com by
irar...@gmail.com
on 3 Jul 2009 at 5:46