Questions about experimental Spice compositor rebase

Mon Feb 29 15:39:59 UTC 2016

Il 29/02/2016 16:01, Yury Shvedov ha scritto:
> Unfortunately, it is bit complex for me, to understand what are you 
> trying to do and why do you dancing with diff's instead of just simple 
> merge.

Sorry for my bad english.
As you not rebase all your commits on top is difficult find all your 
changes from upstream, so I did a quick diff from your branch to the 
last upstream commit corresponding.
Usually I keep do rebase of my patches (with git rebase -i) and other 
patches in development to test always queued to upstream commits to make 
it easier and faster update them, have it ready to post upstream for 
review any version and add the new upstream commits until my patches are 
accepted upstream.
For example: https://github.com/Fantu/Xen/commits/rebase/m2r-testing-4.6
Probably will be good also for your project (I can do it in newer branch).

Look other answer/questions below please.

>
> Anyway, I gave you an edit-access to my repository. If you want, you 
> can work with it directly on your own branch. I hope, this will make 
> things simpler for you.
>
> See answers in quote.
>
> On 02/29/2016 03:32 PM, Fabio Fantoni wrote:
>> Il 29/02/2016 12:26, Yury Shvedov ha scritto:
>>> Hi, Fabio!
>>>
>>> Take look at my latest commit It now merged with latest master 
>>> version and successfully compiles with ./configure 
>>> --enable-spice-compositor.
>>> But unfortunately it doesn't work due to new spice API. I hope, this 
>>> evening it will!.
>>
>> Thanks for your work about it.
>> I make the new diff in other test branch:
>> https://github.com/Fantu/compositor-spice/tree/test2
>> And I have some questions:
>>
>> - src/Makefile.am was removed in newer weston and now unused, I suppose
>>
>> to be removed
>>
> Did I fogot to do it in my repo? Oh yes! My bad! I will!
>>
>> - Makefile.am missed monitor renderer additions, must be added or 
>> monitor renderer is not
>>
>> needed anymore?
>>
> Its doesn't used by spice, so if there no monitor renderer additions 
> in original weston repo, then it is not needed anymore.

Monitor renderer seems something added by you with this project:
https://github.com/ein-shved/compositor-spice/commit/72072ed2671dd400068d48b4f5048855fb066938
There isn't a commit description about, I not understand if it something 
additional for sharing monitor like a new weston plugin I saw (screen 
sharing) or it is different and required for spice compositor.
Can you do a small little explanation if possible please?

>> - src/compositor-rdp.c: I suppose is not needed and not related changes
>>
>> to be removed, right?
>>
> Why? It is just another part of weston. If you don't need it just 
> don't pass --enable-rd-compositor to configure.

I talked only about few lines changed by one of your commit.

>> - src/spice/Makefile.am: I suppose is unused now that thing are added in
>>
>> Makefile.am, to be removed, right?
>>
> Yes, the same as src/Makefile.am
>>
>> Can be the monitor renderer missed/incomplete the cause of "run test" 
>> failed?
>>
> I didn't try tests, so can't answer. Will look at evening.
>>
>> About spice-server api I did't found good docs to make update simply 
>> and fast but with a fast search I found this xspice (similar project 
>> for xorg instead) commit that probably can be faster update some 
>> deprecrated spice functions: 
>> https://cgit.freedesktop.org/xorg/driver/xf86-video-qxl/commit/?id=70884bd353c34c0be23c2b21eec320cd8c637f4f
>>
>> I don't have sufficent free time for try to change it and test 
>> build/use today.
>>
> I spend much time for reading spice source code to understand its API 
> far in 2013. To understand it you have to read sources as I. I 
> remember that in fact spice protocol is - to say simple - drawing API. 
> You can draw stuff from spice-server on spice-client's screen. Anyway 
> we need to learn new api, reading example source code as I did.
>
> I did simple example <https://github.com/ein-shved/qxl-test> then to 
> practice on spice API.
>>
>> After update to newer api I suppose will be good add also a required 
>> spice-server version check in configure based on newer api, I found 
>> this that seems will make fast see at what version was added any api: 
>> https://cgit.freedesktop.org/spice/spice/tree/server/spice-server.syms
>>
> Yes of course we will!
>>
>> Another important note if you don't know it, spice-server recently is 
>> under heavy changes, latest version (0.13.0) is like a "devel 
>> snapshot". Latest stable version that I think is good to use also 
>> with this project for now is 0.12.6.
>>
> Yes, I don't. Is it possible for you to assemble all documents and 
> links on this topic, you found?
>>
>> Thanks for any reply and sorry for my bad english.
>>
>>
>>
>>>
>>> On 02/29/2016 12:22 PM, Daniel Stone wrote:
>>>> Hi Fabio,
>>>>
>>>> On 27 February 2016 at 18:02, Fabio Fantoni <fabio.fantoni at m2r.biz> 
>>>> wrote:
>>>>> Hi, long time ago I saw an interesting project for weston, the spice
>>>>> compositor:
>>>>> https://github.com/ein-shved/compositor-spice
>>>>> It is now abandoned because the developer has been involved in 
>>>>> another
>>>>> project.
>>>>> As no other has continued it, despite my low knowledge and time I 
>>>>> would try
>>>>> to update, test and possibly improve it.
>>>> Great!
>>>>
>>>>> I did a new branch with only 2 commit on top of latest upstream 
>>>>> commit:
>>>>> https://github.com/Fantu/compositor-spice/commits/test
>>>>> and I tried to do a fast rebase on latest upstream commit (1.10) 
>>>>> instead of
>>>>> master (development branch) for decrease the risk regression on first
>>>>> build/use tests:
>>>>> https://github.com/Fantu/compositor-spice/commits/rebase/spice-1.10
>>>>>
>>>>> Solving conflict about configure and makefile parts I have some 
>>>>> doubts (as
>>>>> also reported in the description of each commit):
>>>>> About first commit (Add Spice compositor)
>>>>> https://github.com/Fantu/compositor-spice/commit/f589ab264e80d43fa0853770481b6ddcadf5505b 
>>>>>
>>>>> - in configure.ac some changes seems strange, including LIBS and 
>>>>> CFLAGS that
>>>>> seems "double"
>>>> I think this can be removed. Usually setting LIBS/CFLAGS and
>>>> foo_save_LIBS/foo_save_CFLAGS is used for an AC_CHECK_* call, which
>>>> relies on LIBS and CFLAGS already being set. I guess there may have
>>>> been a call here which has since been removed.
>>>>
>>>>> About the second commit (Monitor renderer)
>>>>> https://github.com/Fantu/compositor-spice/commit/2632b8b8067e46ac69b5ad1bc2164d90ced5e19f 
>>>>>
>>>>> - Makefile things seems fully changed, tried to adapt them but I'm 
>>>>> not sure
>>>>> if I did it correct.
>>>>> - Add -g to AM_CPPFLAGS in Makefile.am is really needed? not added 
>>>>> for now
>>>> No, this is a debugging feature only.
>>>>
>>>>> - add of "-Wl,--wrap=pixman_renderer_init" to LDFLAGS of many 
>>>>> other backend
>>>>> is really needed? not added for now, if needed is good understand 
>>>>> why to add
>>>>> it also to new things added since this start commit done 3 years ago
>>>> This should be solved in a different way if required.
>>>>
>>>>> - src/compositor-rdp.c changes is really needed? if not I'll 
>>>>> remove them
>>>>>
>>>>> I also searched documentation about api and/or internal weston 
>>>>> functions
>>>>> changed any versions but I not found them.
>>>> There is no documentation on the change, no.
>>>>
>>>> As you can see, several functions have changed:
>>>>    - weston_output_finish_frame now takes a struct timespec rather 
>>>> than
>>>> an integer number of milliseconds (trivial conversion)
>>>>    - the output repaint function now returns an integer marking 
>>>> success
>>>> or failure
>>>>    - the compositor interface has now changed to weston_backend, and
>>>> you can see examples of the changes required in commit 954f183e
>>>>
>>>> Hope this helps: just pick out the warnings and errors one by one, and
>>>> try to figure them out - searching git commits for anything relevant
>>>> always helps - until you get something that builds.
>>>>
>>>> Cheers,
>>>> Daniel
>>>
>>> -- Kind Regards, Yury Shvedov
>>
>
> -- Kind Regards, Yury Shvedov

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160229/03eb4ee8/attachment-0001.html>