3 ms·

Too bad I don't have the time to read the long comment thread on there right now. Hopefully I will remember to do so after work. :) Instances like this are re

Too bad I don't have the time to read the long comment thread on there right now. Hopefully I will remember to do so after work. :) Instances like this are really interesting to me, as it seems early on in the thread the commit the broke Photoshop was identified, yet it took them 5 years to finally commit a fix. From the outside it's easy to say the wine devs are lazy/stupid for not having fixed it right away, but I suspect there is a story buried in there that tells why it wasn't that simple.

pjc50 9y ago
I skimmed it and saw at least two attempts that were backed out for various reasons.
- uhhhhhhh 9y ago
  From a rudimentary reading. They introduced a state cache to fix conditions where applications poll continuously for certain states (mouse button press for example). This wasn't thread safe however, and multi-threaded applications could get a previous/incorrect state back from the cache and stop functioning as intended. This was highly visible for photoshop tools where you click and hold, as the separate mouse poller thread would get a state of 'button not clicked" and stop, or in some cases start/stop repeatedly giving a series of dots instead of a line. The patches submitted early were to disable the global cache, which while it fixed the multi-threaded use cases didn't fix the high-polling rate issue that the global cache was attempting to fix. The fix, years later, was to look at the global cache when changing state and invalidate it at the time of state change so any immediate calls to retrieve that state from the cache will not get the previous cached state. There was also another issue that came up in the middle of that thread that was unrelated it seems.
  - web007 9y ago
    > Regressions tend to be high priority for the devs though so this will be fixed eventually. "eventually" is right... This seems to be a failure on the part of the devs to prioritize. AFAIK code that causes a regression should be reverted and fixed properly, not left affecting the product for FIVE YEARS before finding a fix. The burden can not be on every use case to work around a bug that was introduced, the core "fix" is clearly a half-fix and should have been reverted on identification.
    - uhhhhhhh 9y ago
      I mostly agree, but if we want to be pedantic, this didn't regress something so much as fix horrible performance for 90% of apps and introduced a previous non-existent bug into certain applications due to a non-thread safe bug. The solution itself turned out to be simple enough, and the time it took to fix is definitely not excusable considering the effort vs impact IMO.

pjc50 9y ago

I skimmed it and saw at least two attempts that were backed out for various reasons.

uhhhhhhh 9y ago
From a rudimentary reading. They introduced a state cache to fix conditions where applications poll continuously for certain states (mouse button press for example). This wasn't thread safe however, and multi-threaded applications could get a previous/incorrect state back from the cache and stop functioning as intended. This was highly visible for photoshop tools where you click and hold, as the separate mouse poller thread would get a state of 'button not clicked" and stop, or in some cases start/stop repeatedly giving a series of dots instead of a line. The patches submitted early were to disable the global cache, which while it fixed the multi-threaded use cases didn't fix the high-polling rate issue that the global cache was attempting to fix. The fix, years later, was to look at the global cache when changing state and invalidate it at the time of state change so any immediate calls to retrieve that state from the cache will not get the previous cached state. There was also another issue that came up in the middle of that thread that was unrelated it seems.
- web007 9y ago
  > Regressions tend to be high priority for the devs though so this will be fixed eventually. "eventually" is right... This seems to be a failure on the part of the devs to prioritize. AFAIK code that causes a regression should be reverted and fixed properly, not left affecting the product for FIVE YEARS before finding a fix. The burden can not be on every use case to work around a bug that was introduced, the core "fix" is clearly a half-fix and should have been reverted on identification.
  - uhhhhhhh 9y ago
    I mostly agree, but if we want to be pedantic, this didn't regress something so much as fix horrible performance for 90% of apps and introduced a previous non-existent bug into certain applications due to a non-thread safe bug. The solution itself turned out to be simple enough, and the time it took to fix is definitely not excusable considering the effort vs impact IMO.