<dinamic> i have a few questions regarding the keyaccel imlpementation
<dinamic> bieber_, how should i handle keyaccels in a plugin that is shared among views eg. accessable in both lighttable and darkroom for example..
<bieber_> Hmm
<bieber_> I suppose you'd have to add the accelerator to both views
<dinamic> hmm
<dinamic> thats not doable
<dinamic> hmm this is getting messy the more i think of it..
<dinamic> lets say we have a keyaccel shared among vies
<dinamic> views
<dinamic> that will end up in several keyacel paths
<bieber_> I'd have to rewrite the view-switching accel system to allow them to be shared.  Currently a separate list is stored for each view, in this case we'd want a single list withh flags for each accel indicating the views it exists in
<dinamic> and when user configures using the preference pane he need to find 3 places for a single function to be consitetn thru the views
<bieber_> Multiple keys could be a good thing, though
<dinamic> consitensy is better then ability
<dinamic> :)
<bieber_> Hmm
<dinamic> let say left arrow goes to next image in filmstrip
<dinamic> but only darkroom and not in tethering view
<dinamic> because user didnt search all the filmstrip left image
<dinamic> to make it consisten
<dinamic> among the views
<dinamic> couldn't the key be linked somehow ?
<dinamic> oh wait!
<bieber_> The problem there is that that code belongs to the view, doesn't it?
<dinamic> i think i have it..
<dinamic> let's preference pane update all keys
<dinamic> x/y/z/key/action
<dinamic> where last key+action = same
<dinamic> and the key assigned
<dinamic> nah
<dinamic> hmm
<dinamic> grrr
<dinamic> need a smoke
<dinamic> :)
<dinamic> bieber, filmstrip view is in a module now
<dinamic> bieber_, locally ^
<dinamic> :)
<dinamic> ill push it tonite
<dinamic> just cleaning it up now..
<bieber_> Hmm
<dinamic> brb
<bieber_> Auto-reassigning multiple keys seems like potentially confusing behavior
<bieber_> Okay
<dinamic> hmm
<dinamic> yea i figured
<dinamic> maybe wee need too reconsidering the key path structure
* prokoudine__ is now known as prokoudine
<dinamic> we have 3 types of plugins
* prokoudine has quit (Quit: Ex-Chat)
<dinamic> views, plugins (not specific to a view), iop modules
<dinamic> so maybe something like:
* prokoudine (~avp@ppp85-141-150-202.pppoe.mtu-net.ru) has joined #darktable
<dinamic> this mess:
<dinamic> ../views/lightable/mode
<dinamic> ../plugins/collection ../plugins/export
<dinamic> ../iop/highpass
<dinamic> probably something better then iop
<dinamic> like
<dinamic> ../processing modules/higpass
<dinamic> plugins might have a better name
<dinamic> like panels
<dinamic> or somethign
<bieber_> That could work
<dinamic> that removes the view dependecy that exists today
<bieber_> I've got to run for now, I'll be back later
<dinamic> 4 main groups: globals, views,panels, modules
<dinamic> yeap
* bieber_ has quit (Remote host closed the connection)
<dinamic> cya around
* bremner has quit (Read error: Operation timed out)
* bremner (~bremner@yantan.tethera.net) has joined #darktable
* mmiicc has quit (Quit: Leaving.)
* rip-someday (~rip@executiveness.sky.volia.net) has joined #darktable
* paka (~paka@50-90-97-150.res.bhn.net) has joined #darktable
* paka has quit (Remote host closed the connection)
<dinamic> greate
<dinamic> new filmstrip module pushed to post-0.9
* jcsogo (~AndChat@31.Red-83-45-118.dynamicIP.rima-tde.net) has joined #darktable
* KostyaSha (~gentoo@178.120.16.86) has joined #darktable
<prokoudine> dinamic, and I see that you pushed masks too
<bieber> dinamic: We could rearrange the accel groups to group by modules, libs, and views, the problem just then becomes deciding how to handle adding/removing them
<bieber> The reason they're currently grouped by view is so that we can just detach/attach the apropriate groups during a view change, and everything plays nice
<dinamic> biebier, so all modules should have: attach_key_accel() dettach_key_accel() ?
<dinamic> and the use their corresponding module typ api
<dinamic> like libs:
<dinamic> dt_lib_attach_key_accel().
<dinamic> dt_iop_attach_key_accel
<dinamic> dt_view_attach_key_accl
<bieber> Yeah, we could just handle everything by connecting/disconnecting the individual accelerators
<bieber> Here's a thought
<bieber> Rewrite dt_accel_group_connect_by_path so that instead of a group, it accepts a flags argument for the views in which the shortcut should be active
<bieber> Then on view change we can just comb through the flags and connect/disconnect apropriately
<dinamic> prokoudine, yea i merged post-0.9 into it too see how much problem i would get :).. minimal problems..
* rip-someday has quit (Ping timeout: 264 seconds)
<dinamic> bieber, i dont think you should build something view dependent into keyaccels
<dinamic> view.c does the job for us
<dinamic> it loads/unloads plugin when switching view
<dinamic> so in the unload loop disconnect them..
<bieber> That would be nice, it just leaves the problem of remapping
<dinamic> and in the load loop connect
<bieber> The remap code has to be able to tell which accelerators can conflict with each other, so they have to have some notion of the views they're active in
<dinamic> bieber, how soo ?
* jcsogo has quit (Ping timeout: 276 seconds)
<dinamic> bieber, im not following that
<dinamic> s/following/understand
<bieber> When the user remaps a key, the interface has to delete any mappings that can conflict with it
<bieber> However, multiple accelerators can be mapped to the same key, as long as they're only ever active in different views
<bieber> So currently, if you remap a key in the lighttable group, say, it won't delete any matching bindings in the darkroom group
<dinamic> why keep mappings view dependent ?
<bieber> It's necessary if you want the same key to do different things in different views
<houz> speaking of which, didn't you want to add context to the accel strings generated when compiling? it should just be a matter of replacing _(XXX) by C_("accel", XXX)
<dinamic> why would one want that ?
<bieber> If we just check every single accelerator for conflicts, then if you use a key in one view you lose the ability to use it in the others
<dinamic> not if the module isnt loaded in that view
<dinamic> take post-0.9 for example,
<bieber> houz: Is that a macro or a function?  I was having trouble finding the corresponding function to actually look up the transations with context in the UI, I think
<dinamic> rating tool in topbar left box..
<dinamic> i ment the colorlabels..
<bieber> I could find one in glib that took a domain and a context, but not one that just took a context?
<dinamic> let's say that tool have the key_accels for assigning colorlabel
<houz> that the same as _(). just use it in accelstrings_gen.h
<houz> grep for C_() in our sources
* jcsogo (~AndChat@31.Red-83-45-118.dynamicIP.rima-tde.net) has joined #darktable
* paka (~paka@50-90-97-150.res.bhn.net) has joined #darktable
<bieber> dinamic: The problem with that is that we have no programatic way of determining which tools will necessarily be active at the same time.  We can check for conflicts in the currently loaded set of modules, but we have no way of ascertaining what modules may be loaded at the same time in other situations
<bieber> Oh wait, libs already have a views() callback, so I guess we can just use that
<dinamic> :)
<dinamic> yeapp
<dinamic> iop = always darkroom
<dinamic> libs = determined by views()
<dinamic> views =.. views :)
<bieber> Okay, so how does this sound
<bieber> Every lib/iop can have the usual init_accels() callbaack, as well as a connect_accels() and disconnect_accels() that will be called at load and unload
<bieber> And I guess something similar for views as well
<dinamic> bieber, yea thats cleaner then connect/disconnect accesl in gui_init() / cleanup()
* rip-someday (~rip@executiveness.sky.volia.net) has joined #darktable
<bieber> Definitely
<bieber> I probably should have thought of something like that while I was writing all the connect()/disconnect() code in the various modules
<bieber> I'll also have to figure out how boucman's modifications to the various GUI widgets have been working and get them to play nice as well
<dinamic> ahright
* glutko (~glutko@93-96-145-167.zone4.bethere.co.uk) has left #darktable
<dinamic> to the next question about keyaccels
<dinamic> how do one make a window specific mapping ?
<dinamic> for a stubborn example:
<dinamic> if i want enter to activate image under mouse in filmstrip
<dinamic> and i have crop rotate module active
<dinamic> :)
<dinamic> how would one control key to specific window ?
<dinamic> the old impl, pushed it further down a chain
<bieber> Well, there's definitely no official GTK support for something like that
<dinamic> for sure..
<bieber> The solution we decided on at the beginning of the Summer was just to make all shortcuts global, because having different shortcuts activate depending on the mouse position would become exceedingly confusing for the user
<dinamic> but our old key handler handled that
<dinamic> so contextual key accelerators are confusing ?
<dinamic> i guess not
<bieber> Definitely when everything is in the same GTK window
<dinamic> its confusing wanting to do this, but that carries out
<bieber> If you wanted to implement something like that, first you'd have to decide whether you want it to depend on what widget has focus according to GTK, or what widget has the mouse over it
<dinamic> i want it to be controlled as before
<dinamic> like focused module
<bieber> Then you'd just need to connect/disconnect the accelerator in functions attached to the mouse enter/leave or focus in/out signals of the widget
<dinamic> its pretty enough to have something like that
<dinamic> soo
<dinamic> connect_accels(gboolean global)
<dinamic> then..
<dinamic> :)
<dinamic> and let the view connect mouse enter/leave and dispatch (dis)connect_accels(TRUE)
<dinamic> ok, i see its doable
<dinamic> not that we need it for now i guess
<dinamic> but we sure will need it in the future
<bieber> Focus-dependent accelerators remove a lot of the convenience of keyboard shortcuts to begin with, keep in mind
<dinamic> no it doesnt
<dinamic> :)
<bieber> If the user has to focus the module with the mouse to use it anyways, why not just add a button in the module?
<dinamic> if crop backspace is global in view
* jcsogo has quit (Ping timeout: 250 seconds)
<dinamic> how is the old contextual key accels handled ?
<dinamic> like crop for example
<bieber> I _think_ they just activated depending on whether the desired module had focus
<bieber> The old system did everything manually by comparing keycodes on key_pressed events
<dinamic> bieber that what im talking about..
<dinamic> take crop for example
<dinamic> enter crops the image..
<dinamic> if you size the crop are, then leavs for another module
<dinamic> and do something else..
<dinamic> hit enter and the image will be cropped
<dinamic> even if you donr have th crop module in focus
<dinamic> thats really an unexpected beahviour
<dinamic> and thats why global key accels confuses users
<dinamic> for example
<bieber> It shouldn't actually do anything if you haven't changed the crop mask, though
<dinamic> but you get the opint right..
<bieber> Yeah
<dinamic> whatever is hiding behind a bound key is carried out wherever you are in a view
<dinamic> and you have no clue..
<dinamic> why
<bieber> If you're doing a lot of editing with accelerators, though, you _expect_ to be able to carry out functions with accelerators even if their underlying modules aren't currently visible
<dinamic> why crop if you didnt changed the mask ?
<dinamic> :)
<dinamic> take enter for an example
<dinamic> a very good key
<dinamic> to carry out an action
<dinamic> as for crop
<dinamic> as for activate somethign other
<dinamic> etc..
<dinamic> but we could only have enter mapped to one iop module and the whole view cant reuse it
* houz has quit (Quit: Verlassend)
<bieber> I think it would be a useful feature for some modules, but not many
<bieber> In most cases you wouldn't want to have to pull up the module to make an adjustment if you don't need to use the mouse for it
* prokoudine has quit (Quit: Ex-Chat)
<dinamic> bieber, in most cases you WANT to have the modul up front, some of them and more will have overlay ui control etc..
<dinamic> i really dont understand that last argument..
<dinamic> you want to crop a mask without setting the mask ?
<dinamic> and you dont want to drag a slider to change a value ?
<dinamic> im not following you here, my point is, that NO iop module should have GLOBAL key accels
<dinamic> except for enable/disable etc..
<dinamic> the view common thingy for iop
<bieber> In those use cases, though, I'm already going to be bringing up the iop with a mouse, so I wouldn't need the key accels to begin with
<dinamic> then plugins such as history/snapshots
<dinamic> need both local/global key accels
<bieber> That could work
<dinamic> usally plugins will use global key accels..
<dinamic> like history compress etc..
<dinamic> but some want locals to..
<dinamic> as for the mask module im working on
<dinamic> sure i connect/disconnect on module focus but the api should allow the use of it..
<dinamic> for example..
<dinamic> as i mention before, the gboolean flag to connect/disconnect_accels()
<dinamic> and handle the call to them automatically when module is focused unfocused
<dinamic> i think that would solve the cases
<bieber> Okay
<bieber> I should be able to add that along with the new API without too much trouble
<dinamic> and the last question..
<dinamic> the mask editor will have some tools..
<dinamic> to draw on canvas.. some tools have some key accels.. :)
<dinamic> ofcourse
<dinamic> those will be local keyaccels..
<dinamic> hmm
<bieber> Like tools within the module?
<dinamic> yeap
<bieber> What exactly is the mask editor?  An iop or a view that gets loaded like the filmstrip?
<dinamic> mask editor is a plugin that have some basic tools to draw a mask, that you can use with blending in iop
<dinamic> basically
<dinamic> enable local editing kidn of..
<bieber> Hmm
<dinamic> create a mask of face, remove eyes/lips etc from mask, then load the lowpass and blend with mask
<dinamic> to soften out the skin
<dinamic> etc..
<dinamic> just like alpha channel in gimp for layer it will be used as alpha mask for an plugin
<dinamic> the fist tool will be a closed curve that you can drag to a form, theni need some keyaccells just for the editing of the curve..
<dinamic> those need to be local
<dinamic> and should override the globals
<dinamic> let say my tools have grow/shrink key accels..
<dinamic> s and g
* jcsogo (~AndChat@31.Red-83-45-118.dynamicIP.rima-tde.net) has joined #darktable
<bieber> Hmm
<dinamic> i need those local to be trigged even if there is a global s
<bieber> I'd think it would probably be best just to make a special case for those?
<dinamic> no
<bieber> There aren't any other modules that have sub-tools like that, are there?
<dinamic> because its not only there
<dinamic> bieber, the overlays to crop/gnd/vignette can have ?!
<bieber> Wouldn't those just be local to the module itself?
<dinamic> yeap
<dinamic> when module in focus, and is int this or that state
<dinamic> anyway
<dinamic> the local/global connect/disconnect solves it..
<bieber> In that case the module really has to be responsible for managing which of its accels it wants active
<bieber> Well, it solves module focus, it doesn't do anything about having specific tools selected or not
<dinamic> noo
<dinamic> it doesnt
<dinamic> thats internals of module
<dinamic> but as i say, local key accels should override global ones..
<dinamic> thats a mandatory thing
<bieber> Should they both be allowed to exist?
<bieber> If there's a local accel that could possibly be activated in a view, I wouldn't want to allow global accels on the same key
<bieber> Then the user has to know which particular accelerators the module they have in focus exposes, or else they won't know that their global accels just stopped working
<dinamic> yes that is the outcome
<dinamic> and its needed
<dinamic> they cant coexists..
<bieber> Right, so they shouldn't shadow each other, a key should just be considered taken once it has at least one local accel mapped to it
<bieber> I think our conflict resolution code is about to get much more complicated :P
<dinamic> how does gtk handle 2 mapped keys ?
<dinamic> does it carry them out both
<dinamic> or the last added ?
<dinamic> are they stackable ?
<dinamic> that would be sweet
<dinamic> :)