<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi Mark,<div><br></div><div>Sorry for the delay. You caught me when I was on vacation, and I've been busy catching up with things after coming back.</div><div><br></div><div>I haven't had time to play with FrameRetrace much. <br></div><div><br></div><div>First of all I want to reiterate some of my initials comments about FrameRetrace a while back, when it was first announced, on <a href="https://lists.freedesktop.org/archives/apitrace/2015-June/001087.html">https://lists.freedesktop.org/archives/apitrace/2015-June/001087.html</a> .</div><div><br></div><div>On one hand, it doesn't seem that FrameRetrace requires many changes to existing code, per the output of `git diff --diff-filter=DM $(git merge-base master frameretrace)..frameretrace` (seen <a href="https://pastebin.com/E5WNNG8U">https://pastebin.com/E5WNNG8U</a> ), which is good. Still, this needs to be cleaned, as a lot of these changes look spurious, and they should be committed to apitrace master ahead of an eventual frameretrace merge.</div><div><br></div><div>But on the other hand, I still feel that several things were done differently (and unnecessarily redone in cases.) Most noticeably: a completely separate GUI, with a completely different serialization format, using a separate daemon instead of building upon the existing glretrace slave processes.<br></div><div><br></div><div>Even if I can accept a completely separate GUI on apitrace source tree (not ideal, because it will confuse users, and future contributors, but I might come to make peace with it), I can't accept the server as it stands. In particular `daemon/glretrace_globals.cpp` needs to disappear somehow. The server needs to look more like the existing glretrace/d3dretrace/etc (possibly it could even be merged into these), and not duplicate internal code.</div><div><br></div><div>But honestly I feel it's too little too late. Below you're focusing on use cases, whereas my concerns are all but that -- I care about the architecture and ease of maintenance. I don't doubt FrameRetrace is great for users. But if the goal is to merge, then the right architecture should be chosen -- less duplication and more integration.</div><div><br></div><div>Plus fact you think you done all you planed to do is a big red flag to my ears. I'm already treading water with my apitrace maintenance. This will be a butt load of code which I have little interest for. It's all GL specific - a dying API (even if a slow death.) A completely unknown code base. The core ideas are indeed interesting, but they could be more easily achieved within the current components, as opposed to an wholesale re-implementation.<br></div><div><br></div><div>Sorry, but I don't want to give false hopes: merging seems unlikely at this stage...<br></div><div><br></div><div>Jose<br></div><div><br></div><div>PS: It's also surprising to see completely different indentation and code style. I know that's easily fixable nowadays with tools, but to me is a clear sign of how merging this code was a non-goal. If the plan was to merge, why not just get these things right off the bat?</div><div><br></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 16, 2019 at 8:16 PM Mark Janes <<a href="mailto:mark.a.janes@intel.com">mark.a.janes@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello José,<br>
<br>
I'm interested in getting the FrameRetrace UI upstream in the ApiTrace<br>
project. The benefits of doing this would be:<br>
<br>
- When ApiTrace changes the trace format, I have to rebase the<br>
FrameRetrace branch. This is a tedious and time consuming project.<br>
<br>
- Upstream ApiTrace users would benefit from a far more responsive<br>
and fully-featured application for investigating rendering and<br>
performance problems.<br>
<br>
- FrameRetrace would likely benefit from more users/collaborators.<br>
<br>
A possible *disadvantage* of upstreaming FrameRetrace would be<br>
increased overhead due to ApiTrace merge/review process. Is there a<br>
way to incorporate FrameRetrace that wouldn't create additional work<br>
for you?<br>
<br>
I have implemented nearly all of the features that I anticipated for<br>
FrameRetrace. The application is quite usable, and I have gotten<br>
feedback from Mesa devs and OpenGL developers that they have quickly<br>
investigated and solved problems with the tool. OpenGL debugging<br>
features work without metrics, but Mesa developers have enabled<br>
hardware metrics for at least the following platforms:<br>
<br>
- Intel (i965)<br>
- AMD (Radeon)<br>
- Broadcom (VC4)<br>
- Qualcomm (Freedreno)<br>
- Nvidia (Nouveau)<br>
<br>
(caveat: I haven't done testing myself, but I fielded questions<br>
from devs that enabled the platforms and heard that they got<br>
metrics working).<br>
<br>
I have a bucket list of features to add or improve, but I am reluctant<br>
to spend time on them without feedback from users to indicate the<br>
value of the functionality. I would eagerly invest time to make<br>
FrameRetrace more suitable for upstream. Examples might include:<br>
<br>
- Documentation/screenshots/demos<br>
- Feedback from your review<br>
- CMake changes in ApiTrace that I made to enable FrameRetrace (it's<br>
probably not perfect)<br>
- Visibility changes that I made to expose ApiTrace data structures<br>
- Any bugs that end-users encounter with specific application traces.<br>
<br>
What is your opinion of FrameRetrace, and the possibility of<br>
incorporating it into your project? Are there more features that you<br>
would like to see? What information would help you evaluate the<br>
proposal?<br>
<br>
Thanks,<br>
<br>
-Mark<br>
_______________________________________________<br>
apitrace mailing list<br>
<a href="mailto:apitrace@lists.freedesktop.org" target="_blank">apitrace@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/apitrace" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/apitrace</a></blockquote></div></div></div></div></div></div></div>