[compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hey Luca, The performance problem you are experiencing is probably the latency introduced by getting the mousepointer position via the mousepoll plugin and a Mousepoller, which has problems to deliver the position updates smoothly. Mousepoll tries to solve the problem of not calling XQueryPointer () from individual plugins directly, but to centralize those calls, as they are blocking and can sometimes take up to (as tests showed here) 200 nanoseconds to complete. Nevertheless Mousepoll was a bad solution to a real problem - try to simply update the mousecursor position by calling XQueryPointer () in preparePaint (), which is called directly before painting, so each 1 nanoseconds at 60fps and is only enabled via setFunctions () if the showmouse plugin (rulers or particles) is active. Greetinx. ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
[compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hey Luca,
I forgot one important thing in my review:
When you manipulate global OpenGL states you need to set them back to
what they were before after painting - in the ruler-drawing-case you
change the glBlendFunc, but also glLineWidth without restoring them -
drawGuides () could look something like this (with added only on output
with pointer option):
void
ShowmouseScreen::drawGuides (const GLMatrix &transform)
{
unsigned short *color = optionGetGuideColor ();
const intx= mMousePos.x ();
const inty= mMousePos.y ();
const float xf = (float)x;
const float yf = (float)y;
const intthickness= optionGetGuideThickness ();
const float r=
(float)optionGetGuideEmptyRadius ();
CompRect workArea;
const bool restrictGuidesToOutputWithPointer =
optionGetRestrictGuidesToOutputWithPointer ();
if (restrictGuidesToOutputWithPointer)
workArea = screen->getWorkareaForOutput
(screen->outputDeviceForPoint (x, y));
else
workArea = CompRect (0, 0, screen->width (), screen->height ());
const int workAreaX = workArea.x ();
const int workAreaY = workArea.y ();
const int workAreaWidth = workArea.width ();
const int workAreaHeight = workArea.height ();
const int workAreaXPlusWidth = workAreaX + workAreaWidth;
const int workAreaYPlusHeight = workAreaY + workAreaHeight;
/* If the thickness is zero we don't have to draw, but we should
* still mark the region where the guides should be as damaged --
* this is useful when thickness has just been changed.
*/
if (thickness > 0)
{
glLineWidth ((GLfloat)thickness);
GLboolean glBlendEnabled = glIsEnabled (GL_BLEND);
/* we push in any case as we are going to change the blend function */
#ifndef USE_GLES
glPushAttrib (GL_COLOR_BUFFER_BIT | GL_LINE_BIT);
#endif
if (!glBlendEnabled)
glEnable (GL_BLEND);
glBlendFunc (GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
if (restrictGuidesToOutputWithPointer)
{
if (yf - r >= (GLfloat)workAreaY)
drawLine (transform, xf, (GLfloat)workAreaY, xf, yf - r, color);
if (yf + r <= (GLfloat)workAreaYPlusHeight)
drawLine (transform, xf, yf + r, xf,
(GLfloat)workAreaYPlusHeight, color);
if (xf - r >= (GLfloat)workAreaX)
drawLine (transform, (GLfloat)workAreaX, yf, xf - r, yf, color);
if (xf + r <= (GLfloat)workAreaXPlusWidth)
drawLine (transform, xf + r, yf, (GLfloat)workAreaXPlusWidth,
yf, color);
}
else
{
drawLine (transform, xf, (GLfloat)workAreaY, xf, yf - r, color);
drawLine (transform, xf, yf + r, xf,
(GLfloat)workAreaYPlusHeight, color);
drawLine (transform, (GLfloat)workAreaX, yf, xf - r, yf, color);
drawLine (transform, xf + r, yf, (GLfloat)workAreaXPlusWidth,
yf, color);
}
/* we changed the blend function in any way here */
#ifndef USE_GLES
glPopAttrib ();// restore line width and glBlendFunc/GL_BLEND
#else
if (!glBlendEnabled)
glDisable (GL_BLEND);
glBlendFunc (GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
glLineWidth (1.0f);
#endif
}
cScreen->damageRegion (CompRegion (workAreaX, y - thickness / 2 - 1,
workAreaXPlusWidth, thickness + 1));
cScreen->damageRegion (CompRegion (x - thickness / 2 - 1, workAreaY,
thickness + 1, workAreaYPlusHeight));
}
Maybe this saves you some work.
Greetinx,
MC Return
___
compiz mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hello MC Return, and thanks for your detailed review. On 2015-04-07 at 12:42, MC Return wrote: > Nice to see some new Compiz code here. :) More will come. > Regarding your code and possible optimizations: I'm now updating the code following your advice. I was in fact a little worried about high CPU usage; I hope your suggested changes will improve that substantially. I'm keeping the new functionality in the showmouse plugin for the time being. It shouldn't be too hard to separate later, if you prefer. > you > might want to post a merge request against lp:compiz [1] so more people will > notice your work. I certainly will. Thanks, -- Luca Saiu Hypra team (Accessible website in construction) ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
[compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hi Luca, Nice to see some new Compiz code here. :) The rulers are a good idea and a useful, nice addition to Compiz' mouse-related capabilities. You should be rewarded with a review. Regarding your question about putting this functionality into a new plugin: After all it fits into Showmouse as it does exactly that, but then the visual style does not have much in common with the particle-fx this plugin usually uses and I doubt many people will use both effects (particles and rulers) at the same time... I suggest adding shortcuts to be able to toggle the rulers independently from the particles and to optimize the "wrapped" paint functions (preparePaint (), glPaintOutput () and donePaint ()) to not do any redundant calculations. Regarding your code and possible optimizations: You are doing redundant type conversions from float to double, then back to GLfloat again, drawLine () should use floats/GLfloats as arguments instead. In drawGuides () you are also wasting some CPU as you are ignoring the types of variables you use - CompRegion () expects ints, while drawLine () and glLineWidth () (should) expect floats - your optionGet* () return types are int and mousePos.x ()/.y () also. You should try to have this method do the least amount of type conversions. Also you could save the int return values of screen->width () and screen->height () to not have to call them twice in this one method. Setting thickness to 20 for damaging is not necessary at all - simply use the actual int thickness optionGetGuideThickness () gives you - do not always damage the maximum paint region, simply damage the actual paint region instead, which will also improve performance. Hint: You can use the showrepaint plugin for damage region visualization and debugging. You could also check if glIsEnabled (GL_BLEND) and only toggle blending if it is not activated, as this global state might have been activated already by some other plugin. In preparePaint () you are still calling ps.updateParticles () and updating the rotation member variable, in donePaint () you are still damaging/redrawing the particle area, even if there are no particle emitters. Ideas for improvements: The rulers should be separated more from the particles, code-wise and control-wise - triggering rulers should not depend on triggering the particle emitters. The rulers could gently fade in/out, not pop in/out like they currently do - when disabling showmouse with particle emitters enabled the rulers currently wait for the particles to fully disappear... There should be a possibility to configure a maximum length for the rulers, maybe an option to show them only on the output device the cursor is on as well - some users might not like the from one screen end to the other screen's end approach (multimonitor users for example). The rulers could optionally be stippled, animated or use some color-cycling effects, which should of course be configurable. As your other questions regarding sourcecode and repos have already been answered by Stephen and others, I will not reiterate this. I'll take a look at your mouse cursor theme setting plugin soon, as finally getting this missing core mouse functionality for Compiz is awesome, but you might want to post a merge request against lp:compiz [1] so more people will notice your work. Greetinx, MC Return [1] https://code.launchpad.net/~compiz-team/compiz/0.9.12/+activereviews ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hello Dmitry. On 2015-04-01 at 22:08, Dmitry Shachnev wrote: > Which version of Compiz do you use? It was removed from Debian more than > a year ago [1], because its maintainership turned out to be a very difficult > task. compiz --version says "Compiz 0.9.12". The repo was imported from "Compiz version 0.9.12.0+beta1", coming from debian's Alioth repository as far as I was told. > The latest version of Compiz is available in lp:compiz Bazaar branch > (if you don't like Bazaar, you can use git-bzr to clone it). I'll have a look. Thanks. -- Luca Saiu Hypra team (Accessible website in construction) ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hi Luca, On Tue, 31 Mar 2015 18:46:54 +0200, Luca Saiu wrote: >> Gconf support in compizconfig was deprecated a couple of years ago and was >> supposed to have been removed in favour of a >> gsettings back end using dconf. Gsettings is used by Ubuntu's Unity >> desktop, so >> it should be well-supported. > > I am a little confused about this since I'm seeing a different behavior > (on debian), but it's not directly relevant to the two features I've > developed up to this point; I've just added options and let compiz save > them like any other, and indeed it does. Which version of Compiz do you use? It was removed from Debian more than a year ago [1], because its maintainership turned out to be a very difficult task. The latest version of Compiz is available in lp:compiz Bazaar branch (if you don't like Bazaar, you can use git-bzr to clone it). [1]: https://bugs.debian.org/719870 -- Dmitry Shachnev signature.asc Description: OpenPGP digital signature ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hello Stephen, and thanks for your feedback. On 2015-03-30 at 14:07, Stephen M. Webb wrote: > If you're unfamiliar with bzr I can do that for you, but there may > be delays as I try to find the time. I know the basics of bzr but I've never worked with branches; I'll try and get back to you in case of problems. By the way, shall I write to this list or to [email protected] when speaking of development? I'm temporarily including them both. > Gconf support in compizconfig was deprecated a couple of years ago and was > supposed to have been removed in favour of a > gsettings back end using dconf. Gsettings is used by Ubuntu's Unity desktop, > so > it should be well-supported. I am a little confused about this since I'm seeing a different behavior (on debian), but it's not directly relevant to the two features I've developed up to this point; I've just added options and let compiz save them like any other, and indeed it does. >> Another patch containing a new plugin for mouse cursor themes is >> forthcoming. > Nice. That's a long story. I'm writing a separate message. Regards, -- Luca Saiu Hypra team (Accessible website in construction) ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
On 15-03-30 01:30 PM, Luca Saiu wrote: > Hello. Hi. > I've developed a relatively simple addition to the "Show mouse" plugin > to (optionally) draw what we call "mouse guides": horizontal and > vertical lines crossing near the mouse pointer to make its position more > visible to visually impaired users. > > The user can configure the guide thickness, color (including > transparency) and distance from the mouse, using CCSM. OK. > A patch is attached. Please feel free to tell me about any problems, > including about the repo we should be using; we have based our work on > the debian repository on alioth. It was not completely clear to us what > repo is considered "official" for compiz, as several look abandoned. > I'm also slightly concerned about CPU use. Shall I separate the guides > functionality into a new plugin? The official upstream repo for Compiz 0.9 (the version you're targeting since you're writing C++ code) is at [1]. The preferred way to submit changes is to submit a merge proposal, so you can get and respond to feedback from upstream developers. If you're unfamiliar with bzr I can do that for you, but there may be delays as I try to find the time. > A random questions: we are having problems seeing any effect of the > Compiz configuration in gsettings after installation, even if configured > with -DUSE_GSETTINGS=ON; as far as I can see compiz stores its > configuration in gconf. However some code supporting gsettings is > obviously there. Is the functionality not completely implemented, or > are we doing something wrong? Gconf support in compizconfig was deprecated a couple of years ago and was supposed to have been removed in favour of a gsettings back end using dconf. Gsettings is used by Ubuntu's Unity desktop, so it should be well-supported. Unfortunately I'm really not an expert on that area of things, so I can't offer much in the way of suggestions. > Another patch containing a new plugin for mouse cursor themes is > forthcoming. Nice. [1] https://code.launchpad.net/~compiz-team/compiz/0.9.12 -- Stephen M. Webb ___ compiz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/compiz
[compiz] Accessibility: mouse guides for Compiz, and forthcoming contributions
Hello. [Resending the message from a subscribed address] I'm Luca Saiu. I'm part of a small but very motivated team in France, working on accessibility; you might have already exchanged some messages with my colleagues Jean-Philippe Mengual, Ksamak and Raphaƫl Poitevin. We are developing a customized version of debian using Compiz with Emerald and MATE. I've developed a relatively simple addition to the "Show mouse" plugin to (optionally) draw what we call "mouse guides": horizontal and vertical lines crossing near the mouse pointer to make its position more visible to visually impaired users. The user can configure the guide thickness, color (including transparency) and distance from the mouse, using CCSM. A patch is attached. Please feel free to tell me about any problems, including about the repo we should be using; we have based our work on the debian repository on alioth. It was not completely clear to us what repo is considered "official" for compiz, as several look abandoned. I'm also slightly concerned about CPU use. Shall I separate the guides functionality into a new plugin? A random questions: we are having problems seeing any effect of the Compiz configuration in gsettings after installation, even if configured with -DUSE_GSETTINGS=ON; as far as I can see compiz stores its configuration in gconf. However some code supporting gsettings is obviously there. Is the functionality not completely implemented, or are we doing something wrong? Another patch containing a new plugin for mouse cursor themes is forthcoming. Thanks, commit d23ca3dba5882ffaab222f68ff50704caf842e97 Author: Luca Saiu Date: Sun Mar 15 22:48:07 2015 + add mouse guides to the "Show Mouse" plugin Add functionality to the "Show mouse" plugin for drawing guide lines to highlight the mouse pointer. The user can choose the guide thickness, color and transparency. Change the minimum number of emitters from 1 to 0, for the case where we have guides only. CPU use is high (at least on my VirtualBox machines). I don't think the particle engine gets used by mistake. The code is easy enough, but unfortunately we have to manually synchronize a constant in plugins/showmouse/showmouse.xml.in and in plugins/showmouse/src/showmouse.cpp to have the same value. The code generated from the XML file seems to keep the datum private, so I can't easily query it. * plugins/showmouse/showmouse.xml.in: Configuration interface. * plugins/showmouse/src/showmouse.cpp, plugins/showmouse/src/showmouse.h: Here. -- Luca Saiu Hypra team (Accessible website in construction) diff --git a/plugins/showmouse/showmouse.xml.in b/plugins/showmouse/showmouse.xml.in index 0dfa896..07e60f2 100644 --- a/plugins/showmouse/showmouse.xml.in +++ b/plugins/showmouse/showmouse.xml.in @@ -34,6 +34,32 @@ <_long>Toggle the mouse pointer trail. + + <_short>Guide thickness + <_long>How thick mouse guides should be, in pixels. + 0 + 0 + 20 + 1 + + + <_short>Guide empty radius + <_long>Radius of the disk around the cursor which doesn't contain guides. + 20 + 0 + 100 + 1 + + + <_short>Guide Color + <_long>Guide color. + + 0x + 0x0 + 0x0 + 0x + + <_short>Rotation speed <_long>Rotation speed. @@ -51,9 +77,9 @@ <_short>Emitters - <_long>Number of particle emitters. + <_long>Number of particle emitters (0 to disable). 3 - 1 + 0 10 diff --git a/plugins/showmouse/src/showmouse.cpp b/plugins/showmouse/src/showmouse.cpp index 01712d2..073718e 100644 --- a/plugins/showmouse/src/showmouse.cpp +++ b/plugins/showmouse/src/showmouse.cpp @@ -11,6 +11,9 @@ * Copyright : (C) 2009 by Sam Spilsbury * E-mail: [email protected] * + * Copyright (C) 2015 Hypra + * http://www.hypra.fr + * Added guides. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -350,6 +353,12 @@ toggleFunctions (bool enabled) void ShowmouseScreen::genNewParticles (int f_time) { +unsigned int nE = optionGetEmitters (); +if (nE == 0) +{ + ps.active = true; // Don't stop drawing: we may have guides. + return; +} bool rColor = optionGetRandom (); float life = optionGetLife (); float lifeNeg = 1 - life; @@ -373,7 +382,6 @@ ShowmouseScreen::genNewParticles (int f_time) unsigned int i, j; float pos[10][2]; -unsigned int nE = optionGetEmitters (); float rA = (2 * M_PI) / nE; int radius = optionGetRadius (); @@ -562,11 +570,65 @@ ShowmouseScreen::glPaintOutput (const GLScreenPaintAttrib &attrib, sTransform.toScreenSpace (output, -DEFAULT_Z_CAMERA); -ps.drawParticles (sTransform); +drawGuides (sTransform); + +if (optionG
