Migrate from GitHub to SourceForge with this tool. Check out all of SourceForge's recent improvements.
Close

#2770 Locationview

Branch_+_Trunk
closed-accepted
nobody
GUI (476)
5
2009-01-11
2008-12-31
Matthias
No

Location pane is not coincide with diff window in big files. - ID: 2087819

The box in locationview was not propper setted.
In case of big files the min hight was adjust to top. I changed to center. Also the min hight of a difference to min two pixel.

Discussion

  • Matthias

    Matthias - 2008-12-31

    locationview

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-01

    > The box in locationview was not propper setted.
    > In case of big files the min hight was adjust to top.
    > I changed to center.

    You mean the visible area indicator (shaded area)? This could indeed work.

    > Also the min hight of a difference to min two pixel.

    No, this approach does not work. This only makes locationview even more inaccurate.

    Can you please think this for a moment. Lets take simple example:
    - 10000 lines
    - 500 pixels
    --> 20 lines per pixel

    So in this simple case every line in the location pane presents 20 lines in the file. How should we show two 5-line differences having couple of lines between them? Only sane thing is to mark the line as diff. So we have 15-line diff, 10 lines similar and 5 lines of diff.. We can only add two lines different, but it already out of synch...

    We must invent something different.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-01

    > We must invent something different.

    Some kind of zoom feature is probably what we need. Zooming the location pane isn't hard, just change the multiplier. But creating good GUI is the hard part.

     
  • Matthias

    Matthias - 2009-01-01

    >Also the min hight of a difference to min two pixel.
    it makes it a little bit thicker, if you don't like , lets go back to one.
    Would be opposit to your own mind on an other patch.:=(
    For for zooming, I agree:
    I would like to have a tabwindow, with standart one tabpage. on that we have one locationview as now.
    If or value m_pixInLines in class CLocationView is smaller than one, we must add an new page, with size of the shaded area.
    nTopCoord and nBottomCoord will be size of the child tabpage(LocationView), and so on.
    So we will come out of the problem.
    If you create the GUI and guide me a little bit. I will finish this job.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-01

    Yes, I won't apply the change to two pixels. Rest of the patch looks good and I need to just find time to test it. I'm again busy with other application.

    > I would like to have a tabwindow
    Oh, this is new idea.

    If I understood your idea correctly you want to have tabs for different location pane views. So that there is one pane for current location pane, and new tab for less zoomed location pane?

    This is good idea, but I'm afraid this doesn't fit to WinMerge.

    Tabs kind of hide the info. User must either switch between the tabs whole time to see needed info. And switching tab is not easy to do if you need to do it whole time. You cannot easily do if while merging differences.

    The visible area is not important in location pane. You already see it in editor!

    This tab idea gave me another idea. What if we add another bars to the view. These second bars would be zoomed to some smaller zoom factor. Perhaps even 1:1.

     
  • Matthias

    Matthias - 2009-01-01

    should i apply a new one with one pixel?
    Normaly it should not give a probelm, as we have now more than one pixel. In case of many differnce one after the other, the added pixel will be overwritten. If the original is less than 0.5 pixel, we losse this diaplay at all, now and original. so in this case no difference. An other solution can we check the resolution of the screen, low resolution =1
    high =2.
    Tabwindow: Yes at same place of LocationView. We may loose some space for the haeder.
    the tab handling shouldn't be a problem. You will have only as much as needed.
    You can select the global to go rough, than down to make more sensitive. That will be related to the resolution of your srcreen of cource. What ever you do in rough will come to detail, but not opposit. From detail will go only to more detail and of couce also Doc-view. Thats the only way I see to sove the problem.
    If you make all LocationView visible at the time, you have to repaint every child LocationView in case of moveing the main or Doc-view.
    In case of moveing Doc-view we will display the main LocationView only, but also adjust the childs.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-01

    > should i apply a new one with one pixel?

    No need for simple patch as this - I can just pick changes I want to apply.

    > the tab handling shouldn't be a problem
    It really is a problem. I'm regularly merging files with hundreds of diffs. I don't want to switch tabs while I'm copying diffs just to see the status. Changing tab would mean I have to move my hand from keyboard to mouse (so interrupt my work), move mouse cursor, click another tab header to switch tab, see the status and possibly switch back to original tab. In the time it takes to do this I can easily merge several diffs. Tabs simply aren't for this kind of use.

    True, ever mouse-over or context-menu zooming have similar problem. But tabs are worse as they require moving mouse cursor to top of the window.

    Yes, I am strict about usability issues. I've spent years improving WinMerge usability. There are still problems but generally people seem to like the way WinMerge works.

    > If you make all LocationView visible at the time, you
    > have to repaint every child LocationView in case of
    > moveing the main or Doc-view.

    I've thought about location pane updates a lot. Currently one problem is that Locationpane re-draws itself fully every time. Caching diffs in locationpane helped it a bit. But still the repainting is way too slow for large files. There might be thousands of diffs to calculate in million lines in files. So here is one big problem to solve too.

    I think the solution is to learn location pane to re-draw only the changed area. That is if you merge diff in file editor, we only re-draw that one diff's area in location pane. But that isn't trivial to implement.

    Similarly for zoomed location pane we only need to draw the part that changes. The area is always a subset of main location pane. So if main location pane is fast enough, zoomed pane cannot be slow either.

     
  • Matthias

    Matthias - 2009-01-01

    no tab windows?
    you will only have so much pages as needed.
    Thats related to the size. if we talk about 20000 lines a main and two detail are enough.
    But bigger one,... than discuss starts again. :=(
    insteat of tabs we can also use one like our optionsmenue.

    >I've thought about location pane updates a lot. Currently one problem is
    >that Locationpane re-draws itself fully every time. Caching diffs in
    >locationpane helped it a bit. But still the repainting is way too slow for
    >large files. There might be thousands of diffs to calculate in million
    >lines in files. So here is one big problem to solve too.
    programmers issue!
    repaint is done automatically by windows!
    In case you move behind an other window, zooing, or so, no chance you have to repaint complet at all. Moving the cursor, we can repaint our diff area only.

    >Similarly for zoomed location pane we only need to draw the part that
    >changes. The area is always a subset of main location pane. So if main
    >location pane is fast enough, zoomed pane cannot be slow either.
    in redraw you may select the area you have to repaint. (by moveing the shaded area).

    An other way maybe:
    one main LocationView as now, and second where we can adjust the size (zooingfactor)
    If stabel for a while, paint also the difference.
    I mean, selction in mainLV => center childLV. Selcet Child move to your location,...
    Changing with mousewheel ==> put to center ChildLV and resize zooingfactor. Display the factor at top or bottom. Stabel for a while, repaint the childLV for difference.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-01

    This is not about technical implementation. This is about usability. We must first find a good solution that improves usability. Then we can think about how to implement it.

    Tabs are not good GUI for this kind of thing. Tabs are fine for web browser where one tab contains one content, and you don't need to switch between tabs to see different view of the same content. But splitting same actively used content to different tabs is not good for usability. Now, there might be some neat trick to implement usable tabs, but e.g. screenshots would help proving that.

    > repaint is done automatically by windows!
    Don't you think I know this? Lets not get into Windows programming basics. Problem is synchronizing locationpane after changes in files. Part of the location pane data is from backend (list of diffs), part of it is from views (line counts, word-wrap, colors etc). So updating just some part of location pane is not easy.

     
  • Matthias

    Matthias - 2009-01-01

    >We must first find a good solution that improves usability.
    correct
    >Don't you think I know this?
    1000% better than me, not only 100% I'm shure.:)

    But I think do repaint, should be quite easy, as we know the shaded area only there can be a change, right?
    You told LV needs a lot of infos to paint. I think he knows all. We only need a new message what to do.
    I think we can CalculateBlocks(); and than check whitch part we need for painting.
    If CalculateBlocks() takes to long, ok than we need to speed up same, by selecting the data. LV knows the lines for repainting.
    The dispacher should call an new function stead of OnDraw() in this case only.
    Can you tell me the pos how is calling after we change a line, so i can test to speed that up.?

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-01

    I just tested this whole patch and while it works for files having few diffs with enough space between, it breaks if there are several diffs near each other.

    One good Location pane test case is to open conflict file in Testing/Data/big_file.conflict
    That file has 2000+ conflicts in 9000/11000 lines.

    The part of patch that centers the visible area indicator seems to behave well, so I committed it to trunk:
    Completed: At revision: 6265

     
  • Matthias

    Matthias - 2009-01-02

    so far I could test now, our bottelneck is not repaint()
    but SaveBuffForDiff()
    after any change and a delaytime of 1000ms we do saveing.
    in case of realy big files we loose here a long time, as after each change!

    We are saveing to a temp.file.
    than make RunFileDiff()
    with the saved files.

    I think we can inprove here a lot.

    In relation of filesize increase to timer for refresh.
    or restart the timer with any change of doc-view.
    Keep the temp.files till we close the view.
    Save only the file, where we had done chages.

    So we can spare almost 50% of our losttime!

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-02

    > But I think do repaint, should be quite easy

    Well, I haven't managed to implement it in three or so years, so... Of course location pane code improved a lot by my last caching/list changes so maybe it is easier now.

    But the basic problem is that we do it currently like this (simplified):

    1) MergeDocView tells LocationPane to update itself
    2) LocationPane asks difflist from MergeDoc
    3) Locationpane calculates its visual diff list based on difflist from MergeDoc
    4) For each diff locationpane asks from MergeDocView what color the line is

    4) is one big problem I've yet not found good solution. Word-wrapping makes it hard, as lines in the editor don't mach lines in diffs. So location pane cannot just analyze the difflist and determine colors based on linenumbers in diffs.

    If we could simplify this whole procedure, dealing with other problems would become easier.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-02

    > We are saveing to a temp.file.
    > than make RunFileDiff()
    > with the saved files.

    Yes. That is because diffutils code reads data from files, not from memory buffers. We tell diffutils the filenames it compares. And remember there can be additional temp files created for plugins.

    And why not modify diffutils to use memory buffers? Because modifying diffutils is just discouraged, our target is to switch to latest diffutils some day and every additional change makes it harder. New diffutils version would solve some problems but its code looks almost totally different than our version, so whoever does the merge has quite a job to do.

    But yes, giving memory buffers to diffutils would solve many problems. I just think that the order must be:
    1) take new diffutils version to use
    2) modify it with minimal changes to use memory buffers (so the updating would be possible)

    Funny thing with the diffutils update one person sent me a patch for that update. But it was done with VC8. He claimed he had merged our changes, but as it was one huge patch I really had no way to determine that. In other words I had no idea if it works at all. And the author stopped responding so...

     
  • Matthias

    Matthias - 2009-01-02

    give me the patch. I will what I can do.

    Otherwise as your suggestion, we will stop here, and solve other problems.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-02

    The diffutils patch? It is 2 MB zip so I cannot attach it to Sf.net. I can send it to as e-mail attachment?

     
  • Matthias

    Matthias - 2009-01-02

    that's I exspected.
    You have my email acount.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-02

    I don't want to send big attachments before asking (people can be reading mails using slow connections). But, I just noticed that if I only zip the diffutils folder in /Src then th zip is so small I could submit it as a patch item:
    #2482796 Geek's version of diffutils update
    http://winmerge.org/patch/2482796

     
  • Matthias

    Matthias - 2009-01-05

    have you seen the locationview in tortoise. isnt't that a way to do?.
    It zooms auto incase of big files.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-05

    My first thought about zooming was something like that. But then I understood that way is even more confusing for the user.

    If you have long diff, you have no idea about what part the zooming is done. So it is quite useless after all if you really need to know what kind of changes and where are there. Better than nothing, perhaps, but...

    TortoiseMerge is done for merging diffs. There is no editing, no word-wrapping, etc etc. There are dozen of such non-editor compare programs. Even some very popular (and expensive) tools allow very limited editing while comparing/merging.

     
  • Matthias

    Matthias - 2009-01-06

    No, I don't think so.
    Aslong you are outside you move fast, go inside, go smoothly. Not more.
    Of course it breaks also, if we have very.. very big files.

    >If you have long diff, you have no idea about what part the zooming is
    >done.
    Move to doc-view, the locationview gets original again, no zoom.
    Move to locationview it get's zoomed. How much,or ajustable, we have to discuss.

    Only how to hande by keyboard, so as you like. that's first.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-07

    We must think about those very big files too. We can't just implement something and say "this works if you have less than 10000 lines in your files.". And that is why this is not easy - we need to find solution which works for all users not for just some.

    > Move to doc-view, the locationview gets original again, no zoom.
    Play with TortoiseMerge with a large file and see how it really behaves. It is nice idea but it annoys me more than helps:
    - I don't see diffs properly from normal unzoomed location pane
    - I've no idea what part of file I'm looking when zooming so the zooming is just useless many times

    It works when:
    - file is not too long
    - there are no many diffs near each other so one can determine the zoom location

    Result: I never use that zooming.

     
  • Matthias

    Matthias - 2009-01-07

    ok, I think we close this treat here, and put an new item on todo list.
    Anywhere will be a limit of course.
    Will take long time to solve.

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-11

    I've committed the visible area indicator positioning improvement part to 2.12 branch:
    Completed: At revision: 6314

    And as I don't want the other part I'm closing this patch item.

    > Will take long time to solve.
    If it was easy problem we'd have solved this years ago. The real problem with WinMerge is we must handle *ALL* files users may have. Not just some of files. If we plan GUI for just short files lots of users are unable to use WinMerge. If we only think long files, merging short files becomes harder...

     
  • Kimmo Varis

    Kimmo Varis - 2009-01-11
    • milestone: 438013 --> Branch_+_Trunk
    • status: open --> closed-accepted
     

Log in to post a comment.