First thing I have to say is that I like this patch. Vincent van Ravesteijn <[EMAIL PROTECTED]> writes: > I don't know which other problems you exactly meant, but the following > will solve some : > > - Insets without an own color inherit the color from the containing > Inset (e.g. footnotes, tables, captions), > - Selected Insets will be painted with Color_selection incl. math Insets,
That seems to cover what I had in mind. I'll comment on this patch and not on the next one, because it is a sane first step. We can discuss later whether we want to do color blending and how (but I am not sure this is 1.6 scope). > Each Inset has either an own custom color or it has the Color_none > (clear as in ColorCode.h, but we might change this to Color_inherit). > If there is no color specified in its layout or it isn't an > InsetCollapsable, it will have the Color_none as default (changed in > InsetLayout.cpp & Inset.cpp). Good. The only two remaining problems I saw are - math insets previews will keep their background, even when no specific background was really wanted. What is weird is that they become transparent when they are edited or selected. - when selecting several cells of a tabular inset, the contents of the cells is not painted in blue. > Then, given a certain PainterInfo with the selection status and the > color of the containing Inset (or of anyone 'above' if the containing > Inset has Color_none), the method Inset::realizeBackgroundColor > (Inset.cpp) will realize the background color. I'd prefer to keep the backgroundColor(PainterInfo) name, since the 'realize' prefix (used in fonts), tend to imply that we modify the underlying object. Actually, since the method is not intended to become virtual, we could have a method PainterIndo::realizeBackgroundColor(Inset const &) that changes the value of PainterInfo::background_color. This is especially true since the methods uses the variable selected_, which ought to be private. Some additional comments below. JMarc > + bool pi_selected = pi_.selected_; > + Cursor & cur = pi_.base.bv->cursor(); constify. > + bool pi_selected = pi_.selected_; > + Cursor & cur = pi_.base.bv->cursor(); Here too. > /// > + bool selected_; Document the variable. > + if (color_bg != Color_error && color_bg != Color_none) What is this special treatment of Color_error for?
