close
Comments you submit will be routed for moderation. If you have an account, please log in first.
Modify

Opened 4 years ago

Closed 4 years ago

#162 closed bug (fixed)

Color Configuration issues

Reported by: opiopi Owned by: opiopi
Priority: low Milestone: YAM 2.6p1
Component: configuration Version: 2.6
Severity: minor Keywords:
Cc: OS Platform:
Blocked By: Blocking:
Release Notes:

Description (last modified by damato)

The config->Read page have 2 problems:

  1. If "Show text colors" is disabled in the config the poppen buttons should be disabled as well if the page is opened.
  1. At least on OS3.x the colors ar not displayed in the poppen buttons if the page is opened. See also:

http://faq.yam.ch/content/1/5/en/why-does-the-colored-text-gadget-in-the-read-configuration-looks-wrong-the-first-time-i-open-this-sheet.html

The attached patch YAM_COg.diff should fix both issues.

The question is should we enclose the "set(data->GUI..." calls into a

#if defined(__amigaos4__) || defined(__MORPHOS__) || defined(__AROS__)
#endif

pair so it's only applied on OS3.x?

To save #defined statements it could be also applied to all systems and should
not cause negative effects and the execution time should be very fast too.

any suggestions?

Attachments (3)

YAM_COg.diff (2.3 KB) - added by opiopi 4 years ago.
YAM_COs.diff (1.4 KB) - added by opiopi 4 years ago.
Bug162.diff (2.8 KB) - added by opiopi 4 years ago.

Download all attachments as: .zip

Change History (16)

Changed 4 years ago by opiopi

comment:1 Changed 4 years ago by tboeckel

  1. The problem is MUI itself. At creation time the checkmark object has MUIA_Selected==FALSE. When YAM displays the config page it sets MUIA_Selected=FALSE a second time. This set() call is optimized away by MUI and hence no notification is issued.

Since all the CO_Page#?() functions only create the individual pages and set up some notifications I suggest to move the MUIM_MultiSet method call to CO_SetConfig() in YAM_COs.c. Notifications only work for real changes and for user input. Without a change no notification will be triggered.
A different workaround would look like this:

setcheckmark(gui->CH_TEXTCOLORS_READ, TRUE);
setcheckmark(gui->CH_TEXTCOLORS_READ, CE->UseTextColorsRead);

Thus the notification always gets triggered, because either the first or the second setcheckmark() performs a real change.

  1. This is an issue of MUI 3.8. MUI 3.9 seems to have this one fixed. MUI 3.8 seems to perform a similar optimization as above since setting the same pens a second time doesn't change anything. Your proposed change can also be put into CO_SetConfig() instead of CO_PageRead() to let the modification look like this:

set(gui->CA_COLSIG, MUIA_Pendisplay_Spec, &C->ColorSignature);
set(gui->CA_COLSIG, MUIA_Pendisplay_Spec, &CE->ColorSignature);

The first operation is only necessary for MUI 3.8, so this could be enclosed with an #ifdef, but on the other hand it should not hurt if all systems do this.

comment:2 Changed 4 years ago by opiopi

  1. Some other CO_Page#?() functions also call set(...) for setting

some attributes. But i can try to move it into CO_SetConfig() if it's
the better place for such things.

  1. Your suggestion may work but need 2 set() calls for every poppen.

Is it really needed?

let's see what Jens say about that.

comment:3 Changed 4 years ago by opiopi

Seems i should into the source before i reply.

1.
setcheckmark(gui->CH_TEXTCOLORS_READ, TRUE);
setcheckmark(gui->CH_TEXTCOLORS_READ, CE->UseTextColorsRead);
does work fine here. :-)

2.
Your suggestion using the C->Color... does not work here
because both have the same value and does not change anything
so no notification is triggered. :-(

comment:4 Changed 4 years ago by damato

Well, I more or less have the same opinion like Thore.

  1. Move the MUIM_MultiSet method call to CO_SetConfig() in YAM_COs.c and do not use the workaround by setting the attribute to TRUE and immediately back to UseTextColorsRead. This looks like a bad workaround and should be avoided.
  1. Have you tried to do something like? :
    set(gui->CA_COLSIG, MUIA_Pendisplay_Spec, NULL);
    set(gui->CA_COLSIG, MUIA_Pendisplay_Spec, &CE->ColorSignature);
    

However, I would definitly enclose the NULL set into #ifdef and make a large comment that this has been added only because MUI 3.8 is buggy.

comment:5 Changed 4 years ago by damato

  • Description modified (diff)

comment:6 Changed 4 years ago by opiopi

1.
Ok changed again and it works too.
I attach the resulting patch as YAM_COs.diff
but it's not yet complete because of:

2.
Yes of course that was the second try as i saw it was not working.
Seems MUI reject a null pointer here. :-(

Still searching for a (second working) solution...

BTW: Thanks for fixing the wrong formatting here.
i think it was because of some characters in one line!?

Changed 4 years ago by opiopi

comment:7 Changed 4 years ago by opiopi

I found a second (better?) solution for the MUI 3.8 Bug now.

The full patch for this bug would look like the attached file
Bug162.diff.

Maybe we could always activate the page before we call
CO_SetConfig() but i think it would slow down the display time
a lot on some pages so the patch is only applied on page cp_Read
which is currently the onl yone with Poppen Objects.

Is it ok if i commit the patch?

Changed 4 years ago by opiopi

comment:8 Changed 4 years ago by tboeckel

Well, your patch is short, hopefully fixes this issue, includes a big fat warning and applies for one single system only. I'd say you can check it in as it is.

comment:9 Changed 4 years ago by opiopi

Ok i'll commit the patch ASAP.

Maybe you have a 3.x System to test if the issue is fixed for you too!?
At least here on a A2000 with OS3.9 BB2 it works.

The above mentioned website could get a update then too...

comment:10 Changed 4 years ago by opiopi

  • Resolution set to fixed
  • Status changed from new to closed

(In [4685]) * YAM_CO.c: Add a workaround for a MUI 3.8 bug to show the colors for the

Poppen objects on OS3.x.

  • YAM_COs.c: Disable all Poppen objects according to the UseTextColorsRead setting and do some cosmetic changes. This closes #162.

comment:11 Changed 4 years ago by tboeckel

Yes, I have a WinUAE setup at work. I will verify it on Monday.

comment:12 Changed 4 years ago by damato

  • Description modified (diff)
  • Milestone changed from YAM 2.7 to YAM 2.6p1
  • Resolution fixed deleted
  • Status changed from closed to reopened

reopened due to upcoming merge on 2.6p1 branch.

comment:13 Changed 4 years ago by tboeckel

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4998]) * YAM_CO.c: Add a workaround for a MUI 3.8 bug to show the colors for the

Poppen objects on OS3.x.

  • YAM_COs.c: Disable all Poppen objects according to the UseTextColorsRead setting and do some cosmetic changes. This closes #162.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.

This list contains all users that will be notified about changes made to this ticket.

These roles will be notified: Reporter, Owner, Subscriber

  • Frank Weber(Reporter, Owner, Participant)