Bug 107487 - Please add the xterm-256 colour support.
Summary: Please add the xterm-256 colour support.
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: 1.4.2
Platform: Debian testing Linux
: NOR wishlist
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
: 120546 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-06-15 19:24 UTC by Sebastian Andersson
Modified: 2006-07-24 22:49 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
kdebase-konsole-256.patch.gz (3.88 KB, application/x-gzip)
2006-06-04 18:48 UTC, witekfl
Details
kdebase-konsole-schemas.patch.gz (2.34 KB, application/x-gzip)
2006-06-04 18:48 UTC, witekfl
Details
konsole-256.patch (21.08 KB, text/plain)
2006-06-17 16:33 UTC, witekfl
Details
colorspaces-001.patch (25.52 KB, text/x-diff)
2006-06-25 00:25 UTC, Lars Doelle
Details
colorspaces-002.patch (26.38 KB, text/x-diff)
2006-06-26 17:47 UTC, Lars Doelle
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Andersson 2005-06-15 19:24:51 UTC
Version:           1.4.2 (using KDE KDE 3.3.2)
Installed from:    Debian testing/unstable Packages

It would be nice if konsole supported the 256-colour xterm escape sequences:

<ESC>[48;5;<nr>m - set the background to colour <nr> and
<ESC>[38;5;<nr>m - set the foreground to colour <nr>,
where <nr> is between 16 and 232 for a 6x6x6 colour cube and between 232 and 255 for gray colours.
Comment 1 Joseph Smith 2005-08-29 23:23:28 UTC
It may be better to add this to the konsole terminfo database entry and use that instead of xterm.
Comment 2 Kurt Hindenburg 2006-01-22 19:22:14 UTC
*** Bug 120546 has been marked as a duplicate of this bug. ***
Comment 3 Steve Purcell 2006-02-08 02:09:05 UTC
This would also be great for using colour emacs/vim in konsole sessions.

Here's a handy quick way to test whether a terminal supports 256 colours:

 echo -e "\e[38;5;196mred\e[38;5;46mgreen\e[38;5;21mblue\e[0m"

If the words are displayed in the right colours, all is well.
Comment 4 dann 2006-02-21 19:42:18 UTC
If someone want to work on this, what needs to be done is to add support 
for the ESC;38;5;COLORNUMBERm ESC;$8;5;COLORNUMBERm escape sequences for setting the color background and foreground.

The colors 16 ... 256 can be computed with code like this:

for (r=g=b=0, i=16; i< 256; i++) {
   if (i < 232) {
	int j = i - 16;
	int r = j / 36, g = (j / 6) % 6, b = j % 6;
	int red =   (r == 0) ? 0 : r * 40 + 55;
	int green = (g == 0) ? 0 : g * 40 + 55;
	int blue =  (b == 0) ? 0 : b * 40 + 55;
	color.red   = red | red << 8  ;
	color.green = green | green << 8;
	color.blue  = blue | blue << 8;
    } else if (i < 256) {
	int shade = 8 + (i - 232) * 10;
	color.red = color.green = color.blue = shade | shade << 8;
   }

The colors 0 ... 16 are in TEWidget.cpp:base_color_table 2...9 and 12...19
Comment 5 Daniel Butler 2006-04-22 01:39:20 UTC
*** This bug has been confirmed by popular vote. ***
Comment 6 Daniel Butler 2006-04-22 01:42:02 UTC
I've written a piece about 256-color support, and the lack thereof in Konsole.

http://www.yup.com/articles/2006/04/15/ruby-goes-256-color-in-xterm

Comment 7 Kurt Hindenburg 2006-05-01 08:14:48 UTC
echo -e "\e[38;5;196mred" and echo -e "\e[48;5;196mred"

produces a blinking, normal color text in Konsole 1.6.3.

If anyone is working on this, let me know.

BR#37942
Comment 8 dann 2006-05-31 19:25:35 UTC
Adding support for 256 colors in Konsole should be quite easy.
It should not be more than one hour of work to do this Konsole.

I implemented the 256 colors support in gnome-terminal and it took me about 30 minutes after looking at the code for the first time. 

(I do not use either gnome-terminal nor Konsole, but I have an interest in making 256 color terminals available)

If anybody wants to work on this, here are some steps you could follow: 
(obviously you could do things differently, this is just a suggestion)

First change TEWidget.cpp:base_color_table and the code that uses it so that 
colors 0 ... 16 are at indexes 0 .. 16 (this probably is the most time consuming part or the implementation).

Then extend base_color_table to contain 256 colors. The code for computing the RGB components for the colors is given in comment #4. 

Then implement the code that recognizes the ESC;38;5; and ESC;48;5 sequences and changes the fg/bg to the corresponding colors (probably 5-10 lines of code)

Good luck!
Comment 9 witekfl 2006-06-04 18:48:08 UTC
Here is the patch. It adds 256 color support.
There is a problem with transparency and schemas.
Konsole before used 4 colors in color table specially for default background and foreground. Now all 256 colors are taken. I have no idea how to preserve those 4 entries. For now they are removed. I have renumbered colors in schemas:
2 -> 0, 3 -> 1, 12 -> 8, etc. Default background is now color number 7.
Now all schemas look the same: black on white. Color 7 must be redefined there and tranparency set on.

These patches (256 colors and schemas) are against 3.5.3 version.

Witold Filipczyk


Created an attachment (id=16475)
kdebase-konsole-256.patch.gz

Created an attachment (id=16476)
kdebase-konsole-schemas.patch.gz
Comment 10 Lars Doelle 2006-06-06 23:20:21 UTC
> Here is the patch. It adds 256 color support.
> There is a problem with transparency and schemas.
> Konsole before used 4 colors in color table specially for default background and foreground.
> I have no idea how to preserve those 4 entries. For now they are removed. I have renumbered
> colors in schemas: 2 -> 0, 3 -> 1, 12 -> 8, etc. Default background is now color number 7.


Witold,

the colors for default fore- and background are important to preserve.
Removing them creates wrong colours. I for one run "Linux Colors", i.e.
a black default background. 

ESC[40m    set black background
...
ESC[47m    set white background
ESC[49m    set default background color

Hmm, i cannot review you code right now, since bugs.kde.org is down for
maintenance.

And Witold - please excuse me - but who on earth designs such ESC codes
as used in the xterm today? Adding an new escape code is not a mindless fun,
as some xterm author apparently believes. There are a few standards, namely
DEC, ANSI and ECMA on ESC codes, but this sequence is really breaking them all.

ESC[48;5;<Color>m

is wrongly designed in principle. And this is why: The sequence is 

ESC[ <PS> m

where <PS> is a string, i.e. a name. Many such names can be grouped into
one ESC code, e.g.

ESC[ <PS1>; <PS2>; <PS3> m

The CSI-m escape code is associative by definition, i.e. the above is exactly the
same as

ESC[ <PS1> m
ESC[ <PS2> m
ESC[ <PS3> m

So ESC[48;5;<Color>m is anything, but an ANSI or ECMA compatible code
by design. It would break associativity of the escape code, which is a no-no.
Plain and simple.

The code really spells
: 48 - set default background color
:   5 - set blink
: <PS> - for <Color> in most cases an unknown name.

E.g. setting 39 for <Color> would for instance mean "set default foreground" and
not any 6x6x6 color.

One can introduce new names, but one cannot break associativity. No way.
This is polluting the escape space with incompatible codes and nothing else.
The code is already in use and not free. Whoever added the code to the
xterm was clearly not aware about this.

I can understand that you find a 6x6x6 color scheme fine. I do, too. But that
one is a private xterm extension, which is totally wrong designed.

So my opinion is that these code should not be added to the konsole, but
instead the issue should be clarified with the xterm author(s). They have
added an incompatible code, and such should it be coped with, imho.

-lars
Comment 11 Lars Doelle 2006-06-07 02:03:27 UTC
Hmm, a quick follow up...

On Tuesday 06 June 2006 23:20, lars.doelle@on-line.de wrote:

> The code really spells
> : 48 - set default background color
> :   5 - set blink
> : <PS> - for <Color> in most cases an unknown name.


Here i am wrong - 48 is not defined, at least not for VT100 and its successors.
Neverthenless it does not invalidate my argumentation. See e.g.

http://vt100.net/docs/vt100-ug/chapter3.html#S3.3.2

which is very clear about the association. Thus 48 would be ignored, 5 would set
blink and any defined value for PS would legal be interpreted as such by virtually
any terminal which emulates a VT100 based set of ESC codes. The extension is
incompatible.

But - ECMA-48:

http://www.ecma-international.org/publications/files/ecma-st/ECMA-048.pdf

See 
Comment 12 awendt 2006-06-07 07:40:09 UTC
> So a parameter substring is 0-9 and the colon. The semicolon separates
> sub-parameters. Thus 48:5:<Color> would be one sub-parameter, and
> 48;5;<Color> many independent, each having an independent meaning in case
> of a selective parameter.


I think you may be onto something here with the colons... I was able to find 
ITU T.416 (which is the same as ISO 8613-6) and it says:

--- snip ---

The parameter values 38 and 48 are followed by a parameter substring used to 
select either the character foreground “colour value” or the character 
background “colour value”.

A parameter substring for values 38 or 48 may be divided by one or more 
separators (03/10) into parameter elements, denoted as Pe. The format of such 
a parameter sub-string is indicated as:

         Pe : P ...

Each parameter element consists of zero, one or more bit combinations from 
03/00 to 03/09, representing the digits 0 to 9. An empty parameter element 
represents a default value for this parameter element. Empty parameter 
elements at the end of the parameter substring need not be included.

The first parameter element indicates a choice between:

           0   implementation defined (only applicable for the character 
foreground colour)
           1   transparent;
           2   direct colour in RGB space;
           3   direct colour in CMY space;
           4   direct colour in CMYK space;
           5   indexed colour.

If the first parameter has the value 0 or 1, there are no additional parameter 
elements.

If the first parameter element has the value 5, then there is a second 
parameter element specifying the index into the colour table given by the 
attribute “content colour table” applying to the object with which the 
content is associated.

--- snip ---

The separator character 03/10 they use is a colon, not a semicolon... I wonder 
if the xterm implementation was based on an improper reading of the standard?

My reading actually makes me think that the correct format would be 38;5:x to 
set colour x. In other words, a semicolon, then a colon. Here is my thinking:

1. The first sentence starts with "The parameter values 38 and 48 are followed 
by a parameter substring used to [...]". And ECMA-48 section 5.4.2 tells us 
that a parameter string is a series of parameter substrings separated by 
03/11 (semicolon). So since 38 and 48 are followed by a parameter substring, 
it should have a semicolon after it.

(You may object to this first point by saying that the ITU standard and ECMA 
standard possibly define terms like "parameter substring" differently, but 
ITU T.416 doesn't define such things at all. Instead it says they 
are "defined in ISO 6429", and the ECMA-48 "brief history" section (page 5)  
seems to say that ECMA-48 and ISO 6429 are the same document.)

2. The quoted text above also calls the 5 in 38;5:x the "first parameter 
element" in the list separated by 03/10 (colon). If it's the first in a list 
of two "parameter elements", then the first colon should be after it to 
separate these two elements.

If my reading is correct, then the ISO/ITU standard has already broken 
the "associativity" of the SGR parameters when used with 38 and 48. In light 
of that, I think you might as well support setting colour the Xterm way as 
well as the ISO standard way, since both ways are non-associative and by now 
a lot of software expects things to work the Xterm way.

Xterm also supports redefining the colours in the 256-colour palette, with a 
command like this:
echo -e '\033]4;65;rgb:ff/00/00\033\\'

The 6x6x6 colour cube is just a convenient default. If Konsole gets 256-colour 
support, it might be good to support those kinds of palette-setting 
sequences, too.
Comment 13 Lars Doelle 2006-06-07 13:33:42 UTC
Andy,

i certainly do not want to overdo it, and i certainly do not want to stand in the
way of adding a useful feature.

I checked "ctlseqs.ms" from the xterm distribution meanwhile which defines the
sequence as follows:

---
                 Ps = 1 0 7  -> Set background color to White
...                
            If 88- or 256-color support is compiled, the following  apply.
                 Ps = 3 8  ; 5  ; Ps -> Set foreground color to the second Ps
                 Ps = 4 8  ; 5  ; Ps -> Set background color to the second Ps
---

This means, these xterm-"Ps" can only occure within one escape code.

Given that 38/48 are reserved selectors, chances of a collision are actually low.

No one ever fully implemented ECMA-48, which is a mixture of terminal and
printer codes, really. So i would only take it as a guide line, while upholding it
as good as possible.

The problem with the ECMA-48 definition is, that the colon used in a sub-parameter
would likely break any VT100 alike parser. May be, that is the reason why these
anomalous "Ps" where chosen for xterm.

I'll thus write to Thomas Dickey, the xterm maintainer, to learn about his point of view.

Additionally, i'll review the patch, meanwhile, which looks fine on the first glance,
in particular the change to parser material in TEmuVt102.cpp looks good. This
leaves default colors.

Problem here was most probably, that it breaks the format of the screen and history,
which uses one byte per color. Now 256+2 do not fit into a byte anymore. Solution
would be to extend the space per color to 2 bytes. The color have to be kept in indexed
format in history and screen to allow proper interpretation of the default colors and cannot
be translated to RGB. Perhaps one should reserve one byte to indicate the color space,
perhaps a few bits are left in the rendition attribute for that purpose...

> If my reading is correct, then the ISO/ITU standard has already broken 
> the "associativity" of the SGR parameters when used with 38 and 48. In light 
> of that, I think you might as well support setting colour the Xterm way as 
> well as the ISO standard way, since both ways are non-associative and by now 
> a lot of software expects things to work the Xterm way.
> 
> Xterm also supports redefining the colours in the 256-colour palette, with a 
> command like this:
> echo -e '\033]4;65;rgb:ff/00/00\033\\'
> 
> The 6x6x6 colour cube is just a convenient default.


Is the 6x6x6 color cube predefined, btw.?

> If Konsole gets 256-colour  
> support, it might be good to support those kinds of palette-setting 
> sequences, too.


Hmm, changing the color format has some consequences. E.g. are colors of text already
written supposed to be preserved or changed when the interpretation changes via ESC]4?
How is transparency to be dealt with in light of the extended color space?

-lars
Comment 14 awendt 2006-06-07 21:01:23 UTC
> Is the 6x6x6 color cube predefined, btw.?


Yeah, it's in effect when Xterm first starts up. The colours are defined in  
256colres.h in the Xterm source. The first 16 are the regular colours 
(aliases, not duplicates, so I can get green with either 32 or 38;5;2, and 
the sequence \033]4;2;rgb:00/00/ff\033\\ will redefine text printed with 
either of them to be blue). The next 216 indices are for the 6x6x6 colour 
cube. The last 24 are a greyscale ramp.

> Hmm, changing the color format has some consequences. E.g. are colors of
> text already written supposed to be preserved or changed when the
> interpretation changes via ESC]4? How is transparency to be dealt with in
> light of the extended color space?


The behaviour of xterm is that the colours are updated everywhere when a 
palette change occurs.

How would transparency be a problem?
Comment 15 Lars Doelle 2006-06-08 00:50:54 UTC
On Wednesday 07 June 2006 21:01, awendt@putergeek.com wrote:

> > E.g. are colors of
> > text already written supposed to be preserved or changed when the
> > interpretation changes via ESC]4? How is transparency to be dealt with in
> > light of the extended color space?
>  
> The behaviour of xterm is that the colours are updated everywhere when a 
> palette change occurs.


One could do it this way, problem is, that the screen has to be fully redrawn.
Redrawing normally uses a differential algorithm based on the screen contents,
which is not changed by that operation. As 256 colors are likely to be set in
one go, this would mean 256 redraw operations. It would have to be integrated
somehow in the regular bulk operations, which likely delays the regular drawing.

> How would transparency be a problem?


Each color has a transparency attribute, too, in the konsole. They can be set individually
in the schema definitions. Not too big an issue. Another point is a rendition attribute,
intense colors. Normally, the konsole would thus have 2*(256+2) colors. I'm not sure
whether it is better to fully integrate the 256 color feature or to tack it at aside.

-lars
Comment 16 witekfl 2006-06-10 14:10:20 UTC
>            1   transparent;
>            2   direct colour in RGB space;

                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
This feature would be nice.

You may replace uint8 with uint16 for fg and bg or do whatever else
if you want 256 colors.
Now konsole is incompatible with xterm-256 and IMHO should not set
TERM=xterm
I use ELinks in 256 colors mode on rxvt and xterm, and ELinks in 256 colors
looks bad on konsole.
IMHO konsole ouught to have seperate entry in terminfo.

What do you think?

Witek
Comment 17 Lars Doelle 2006-06-11 14:10:16 UTC
Witek,

> Now konsole is incompatible with xterm-256 and IMHO should not set
> TERM=xterm
> I use ELinks in 256 colors mode on rxvt and xterm, and ELinks in 256 colors
> looks bad on konsole.
> IMHO konsole ouught to have seperate entry in terminfo.
> 
> What do you think?


The konsole has a terminfo entry. It could use a review, perhaps.
You can configure the TERM variable in the konsole's session profiles,
if you need to.

The central problem with using a separate entry for konsole is that it is
not honored by every application, in particular mc, which only offers some
vital features when the terminal is believed to be an xterm. It is not that bad
anymore, meanwhile it became ncurses-aware and apparently now offers a
secondary screen to non-xterms, too. It does not provide mouse input, though,
then, making the mc a "mouseless commander".

It would in fact nice to have the konsole separated from the xterm terminfo
entry, as time and again new escape codes show up in xterm and the related
terminfo, breaking the konsole with other programs, vim in particular.

As i said, coping well with mc is the main hurdle of such a step. It is not that
i want to blame mc. They have had quite some problems with broken termcap
and terminfo entries on other Unixes and hardcoded originally quite some
stuff. It became better though.

You can help by digging a bit into the issue, perhaps one can make mc
recognise the mouse introducing kmous=\E[M and likely entries. Again,
i believe making konsole export TERM=konsole by default is a very good
idea.


Witold, i was in contact with Thomas Dickey, the xterm maintainer, about
the strange escape sequences and the result is, that he simply did not had
ISO 8613-6 at hand, simply because it costs $$$. He was aware about the
issues, but expected no one to use the codes beside xterm.

Now since the standard appears to broken anyway, and has thus various
problems in itself, the codes introduced by Thomas are perhaps the best
we can get. You modification in the scanner are absolute fine with me and
in conformance with the xterm handling.

But the newly introduced colour space does not fit well with the konsole,
it is default colours and intense colours, that suffer. I have not yet found
time to work through this, but i believe, one probably needs to change the
class ca, and extend the fore- and background colors to UINT16 to cope
with issue, see TECommon.h

One would then have (2+256) BASE_COLORS and (INTENSITY*BASE_COLORS)
total TABLE_COLORS. This change would go through in most cases, i think,
but i'm not sure if in all. Would you try that?


-lars
Comment 18 witekfl 2006-06-11 18:29:31 UTC
On Sun, Jun 11, 2006 at 12:10:27PM -0000, Lars Doelle wrote:
> The konsole has a terminfo entry. It could use a review, perhaps.
> You can configure the TERM variable in the konsole's session profiles,
> if you need to.


I didn't know.

> Witold, i was in contact with Thomas Dickey, the xterm maintainer, about
> the strange escape sequences and the result is, that he simply did not had
> ISO 8613-6 at hand, simply because it costs $$$. He was aware about the
> issues, but expected no one to use the codes beside xterm.
>
> Now since the standard appears to broken anyway, and has thus various
> problems in itself, the codes introduced by Thomas are perhaps the best
> we can get. You modification in the scanner are absolute fine with me and
> in conformance with the xterm handling.


Lars,
what about to go one step further and use ESC[38;5;2;<red>;<green>;<blue>m
codes? foreground and background as UINT16 gives 65536 colors and only two
bytes more per cell.

> But the newly introduced colour space does not fit well with the konsole,
> it is default colours and intense colours, that suffer. I have not yet found
> time to work through this, but i believe, one probably needs to change the
> class ca, and extend the fore- and background colors to UINT16 to cope
> with issue, see TECommon.h
> 
> One would then have (2+256) BASE_COLORS and (INTENSITY*BASE_COLORS)
> total TABLE_COLORS. This change would go through in most cases, i think,
> but i'm not sure if in all. Would you try that?


I don't know which formula to use it to define intense 256 colours.
I can code it, but next week, not now.
Comment 19 Lars Doelle 2006-06-11 22:34:36 UTC
Witold,

> what about to go one step further and use ESC[38;5;2;<red>;<green>;<blue>m
> codes? foreground and background as UINT16 gives 65536 colors and only two
> bytes more per cell.


It should read ESC[38;2;<red>;<green>;<blue>m, i guess.

While you're at it anyway, please make up your mind how to implement this. Colours
are all indexed right now. It would perhaps be most natural to define something like
colour spaces. You could consider a UINT32 for the fg/bg colour in class ca leaving
you a full byte for defining the colour space identification.

(Hmm, you need to check for "_fg" and "_bg" in TEScreen.h, too, when changing class
ca as there are some state variables for the current and the saved colours.)

If you choose to introduce colour spaces, you could basically leave schema.cpp as it was.
Using colour spaces would allow to actually locate the colour interpretation into function
used by TEWidget::drawAttrStr, where you could extend the colour indices and RGB
colours algorithmically. As true RGB color would be provided, too, there were not much
need to set the 256 to anything but the default. Just an idea.

I note that the 256 color space includes 16 "system colors", actually it appears to be
intended to include the "normal" colour space.

> I don't know which formula to use it to define intense 256 colours.


In a HSV-model, i would increase the V, i.e. multiply each RGB colour component by
some same factor > 1. This might mean to dim the "regular" colours first.

If you are using an 6x6x6 colour cube anyway, this is best done algorithmically instead
expanding a table, as this eases later adjustments. This concerns both the tables
'color_name' and 'default_table' in schema.cpp. The default_table is actually just a
default in cases when no schema is installed. The schema.cpp was defined for the
system colours, only, i'm not certain whether it is good idea to extend the schemata
beyond these.
> I can code it, but next week, not now.


No need to hurry. 256 colours are in xterm since 1999, so konsole can almost certainly
wait one week more. Please ask if you run into problems.

-lars
Comment 20 witekfl 2006-06-17 16:33:14 UTC
Here is the patch. Memory usage is threefolded.
fg_rgb and bg_rgb is used only in 256/true colour mode. Mode is indicated by
fg == 128 or bg == 128 respectively. I didn't init colors_256
programmatically, because I didn't know where to do it.
rgbused is used in many places, because of usage of color_table there.
Please, don't ask me to rewrite it again.

Witek


Created an attachment (id=16652)
konsole-256.patch
Comment 21 Lars Doelle 2006-06-24 05:09:25 UTC
Witek,

> Here is the patch.

Thanks for the patch. i've not yet tried out the code, but the patch looks good.

> Memory usage is threefolded.

Don't think this would be an issue. People gave up blaming the konsole to be a
space hog long time ago, already...

> fg_rgb and bg_rgb is used only in 256/true colour mode. Mode is indicated by
> fg == 128 or bg == 128 respectively.

I'll try to merge f and fg_rgb into one 32bit value, using one of the bytes for the
colour space designator, but preserving the overall logic you use. Perhaps i
can preserve the 256 colour indices until drawing this way, too.

> I didn't init colors_256 programmatically, because I didn't know where to do it.

Ok, i'll check for some hook. 256 colour mode is ways nicer now, as you maintain
the system colours. Schemata are not affected any longer. Yep. The patch is
much nicer and exactly to the point. 

> rgbused is used in many places, because of usage of color_table there.

Some stuff is particular to the system colours, e.g. transparency and does
not really apply to 1byte and 3byte colours. That's ok. I'll check the places,
but it all looks reasonable on the first glance, perhaps beside the stuff related
to bold.

> Please, don't ask me to rewrite it again.

This was very brave, Witek ;^) and the result was worth the effort. Again, thanks
for the patch. I think, this basically is it. I'll try to finalise it. I notice a few omission,
most particular save and restore of colours on the stack (sa_cu_*). Please leave
me a bit time to take care for it. I'll post a patch.

-lars
Comment 22 Lars Doelle 2006-06-25 00:25:24 UTC
Witek, All,

find attached a patch along the lines of Witek's work and my
previous reply.

The patch introduces colour spaces to represent colours in
screen now, thereby localising the necessary changes.

Though the patch is relatively local and survived my testing,
it is pretty intrusive and has the potential to break some of
the rendition hacks, that i'm not aware of. The changes i find
critical are all in TEWidget.C. In particular, I could make no
sense off:

    image[i].c = 0xff; //' ';
    image[i].f = 0xff; //DEFAULT_FORE_COLOR;
    image[i].b = 0xff; //DEFAULT_BACK_COLOR;
    image[i].r = 0xff; //DEFAULT_RENDITION;

so i reverted this one to the original code.

-lars


Created an attachment (id=16781)
colorspaces-001.patch
Comment 23 Kurt Hindenburg 2006-06-26 01:45:37 UTC
Do you want to try to get this into the 3.5.x branch?  We have until Jul 10 for 3.5.4.
Comment 24 Lars Doelle 2006-06-26 15:34:18 UTC
Kurt, All,

> Do you want to try to get this into the 3.5.x branch?  We have until Jul 10 for 3.5.4.


The patch was not intended for rapid release, but it is pretty conservative and is likely,
if at all, to break only visual appearances in unusual cases. And if, these would all be
one-liners.

I found, that the patch introduces a bug that becomes visible in the transparent-MC
schema, i'll try to fix that one. I did not test the 3byte colour codes, but will do so and
add it to the script in the test suite. I did not test printing, either, yet. I cannot test XIM.

So from my side, the patch could use three or four more tests, and i would appreciate
if you all test your favourite, most obscure feature with it, either. If no other complains
come up, it could safely go into 3.5.4 from my side.

-lars
Comment 25 Lars Doelle 2006-06-26 17:47:28 UTC
All,

i think, this is it.

i fixed the transparent-MC bug, i wrote about earlier, tested 3 byte colours, added a
related testscript and tested printing.

Try the ./tests/ct2 (or ./tests/colortest.sh, if you can stand blinking)
and the ./tests/color-spaces.pl, all perhaps using various schemata.

Printing is at least not made worse, as i discovered a bug, but it was already
there. To reproduce it, set the "Linux color" schema, say "man man" and
print using "Pixel for pixel" and "Printer unfriendly" option, to find the highlighted
text not being printed. I did not dig into further it, may be, it is only because it prints
boldly white on white ;^). Default colors should perhaps be treated differently
while printing.

The only other thing, i believe is somehow fishy, is the "0xff" stuff reverted by me in
TEWidget::clearImage. If it ever meant anything at all, which i doubt meanwhile, it
should have broken something that i'm not not aware of, but i don't see what and it
does not have any negative effect in any test. I suppose, the "0xff" is a is a left-over
of some testing, really. Actually, the values set in clearImage should not really have
an effect as long as not paintEvent happens before a setImage comes, which should
happen only during program startup, perhaps, in which case the current setting is a
better default. As no special treatment of "0xff" in setImage is done (and what should
this be?), i would consider this one save either.

-lars


Created an attachment (id=16793)
colorspaces-002.patch
Comment 26 Kurt Hindenburg 2006-07-02 02:06:49 UTC
SVN commit 556955 by hindenburg:

Add 256 color support.
Patch by Lars Doelle. Thanks!

Will forward port shortly.

BUG: 107487


 A             README.moreColors  
 M  +150 -21   konsole/TECommon.h  
 M  +14 -39    konsole/TEScreen.cpp  
 M  +9 -10     konsole/TEScreen.h  
 M  +22 -21    konsole/TEWidget.cpp  
 M  +51 -34    konsole/TEmuVt102.cpp  
 A             tests/color-spaces.pl  
Comment 27 Kurt Hindenburg 2006-07-03 18:57:30 UTC
SVN commit 557629 by hindenburg:

Add 256 color support.
Patch by Lars Doelle. Thanks!

CCBUG: 107487



 A             README.moreColors  
 M  +150 -21   konsole/TECommon.h  
 M  +14 -39    konsole/TEScreen.cpp  
 M  +9 -10     konsole/TEScreen.h  
 M  +22 -21    konsole/TEWidget.cpp  
 M  +51 -34    konsole/TEmuVt102.cpp  
 A             tests/color-spaces.pl  
Comment 28 bluer00m 2006-07-24 08:39:43 UTC
> The separator character 03/10 they use is a colon, not a semicolon... I wonder 
> if the xterm implementation was based on an improper reading of the standard? 

Yes, it was.  I was working from T.416 and apparently completely missed that they were using : instead of ;.  In fact, this is the first time I've seen anyone mention that.

On the plus side, I _tried_ to follow the only relevant standard I could find...
Comment 29 Lars Doelle 2006-07-24 22:49:38 UTC
On Monday 24 July 2006 08:39, bluer00m wrote:
> > The separator character 03/10 they use is a colon, not a semicolon... I wonder 
> > if the xterm implementation was based on an improper reading of the standard? 
> 
> Yes, it was.  I was working from T.416 and apparently completely missed that they
> were using : instead of ;.  In fact, this is the first time I've seen anyone mention that. 


How to do think one should cope best with the issue, bluer00m?

-lars